Skip to content

Commit

Permalink
fix(Symbol): deprecate for_global in favour of for_key and for_api
Browse files Browse the repository at this point in the history
`for_global` was documented as `for_key` but implemented as `for_api`.

Closes denoland#1323
  • Loading branch information
lrowe committed Sep 17, 2023
1 parent 0f06970 commit a198203
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
36 changes: 36 additions & 0 deletions src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ extern "C" {
isolate: *mut Isolate,
description: *const String,
) -> *const Symbol;
fn v8__Symbol__For(
isolate: *mut Isolate,
description: *const String,
) -> *const Symbol;
fn v8__Symbol__ForApi(
isolate: *mut Isolate,
description: *const String,
Expand Down Expand Up @@ -58,6 +62,38 @@ impl Symbol {
/// To minimize the potential for clashes, use qualified descriptions as keys.
/// Corresponds to v8::Symbol::For() in C++.
#[inline(always)]
pub fn for_key<'s>(
scope: &mut HandleScope<'s, ()>,
description: Local<String>,
) -> Local<'s, Symbol> {
unsafe {
scope
.cast_local(|sd| v8__Symbol__For(sd.get_isolate_ptr(), &*description))
}
.unwrap()
}

/// Retrieve a global symbol. Similar to `for_key`, but using a separate
/// registry that is not accessible by (and cannot clash with) JavaScript code.
/// Corresponds to v8::Symbol::ForApi() in C++.
#[inline(always)]
pub fn for_api<'s>(
scope: &mut HandleScope<'s, ()>,
description: Local<String>,
) -> Local<'s, Symbol> {
unsafe {
scope.cast_local(|sd| {
v8__Symbol__ForApi(sd.get_isolate_ptr(), &*description)
})
}
.unwrap()
}

#[deprecated(
since = "0.77.0",
note = "This was documented as `for_key` but implemented as `for_api`"
)]
#[inline(always)]
pub fn for_global<'s>(
scope: &mut HandleScope<'s, ()>,
description: Local<String>,
Expand Down
14 changes: 12 additions & 2 deletions tests/test_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7374,14 +7374,24 @@ fn symbol() {
let s = v8::Symbol::new(scope, Some(desc));
assert!(s.description(scope) == desc);

let s_pub = v8::Symbol::for_global(scope, desc);
let s_pub = v8::Symbol::for_key(scope, desc);
assert!(s_pub.description(scope) == desc);
assert!(s_pub != s);

let s_pub2 = v8::Symbol::for_global(scope, desc);
let s_pub2 = v8::Symbol::for_key(scope, desc);
assert!(s_pub2 != s);
assert!(s_pub == s_pub2);

let s_api = v8::Symbol::for_api(scope, desc);
assert!(s_api.description(scope) == desc);
assert!(s_api != s);
assert!(s_api != s_pub);

let s_api2 = v8::Symbol::for_api(scope, desc);
assert!(s_api2 != s);
assert!(s_api2 != s_pub);
assert!(s_api == s_api2);

let context = v8::Context::new(scope);
let scope = &mut v8::ContextScope::new(scope, context);

Expand Down

0 comments on commit a198203

Please sign in to comment.