Skip to content

Commit

Permalink
Deprecate JsStatic in favor of #[wasm_bindgen(thread_local)] (#4057)
Browse files Browse the repository at this point in the history
Removed `impl Deref for JsStatic` when compiling with `cfg(target_feature = "atomics")`, which was unsound.
  • Loading branch information
daxpedda authored Aug 6, 2024
1 parent 0b1cce6 commit acfd1ee
Show file tree
Hide file tree
Showing 23 changed files with 266 additions and 127 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@
* Added an experimental Node.JS ES module target, in comparison the current `node` target uses CommonJS, with `--target experimental-nodejs-module` or when testing with `wasm_bindgen_test_configure!(run_in_node_experimental)`.
[#4027](https://github.com/rustwasm/wasm-bindgen/pull/4027)

* Added importing strings as `JsString` through `#[wasm_bindgen(static_string)] static STRING: JsString = "a string literal";`.
* Added importing strings as `JsString` through `#[wasm_bindgen(thread_local, static_string)] static STRING: JsString = "a string literal";`.
[#4055](https://github.com/rustwasm/wasm-bindgen/pull/4055)

### Changed
Expand Down Expand Up @@ -129,6 +129,12 @@
* Filtered files in published crates, significantly reducing the package size and notably excluding any bash files.
[#4046](https://github.com/rustwasm/wasm-bindgen/pull/4046)

* Deprecated `JsStatic` in favor of `#[wasm_bindgen(thread_local)]`, which creates a `std::thread::LocalKey`. The syntax is otherwise the same.
[#4057](https://github.com/rustwasm/wasm-bindgen/pull/4057)

* Removed `impl Deref for JsStatic` when compiling with `cfg(target_feature = "atomics")`, which was unsound.
[#4057](https://github.com/rustwasm/wasm-bindgen/pull/4057)

### Fixed

* Copy port from headless test server when using `WASM_BINDGEN_TEST_ADDRESS`.
Expand Down
2 changes: 2 additions & 0 deletions crates/backend/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ pub struct ImportStatic {
pub js_name: String,
/// Path to wasm_bindgen
pub wasm_bindgen: Path,
/// [`true`] if using the new `thread_local` representation.
pub thread_local: bool,
}

/// The type of a static string being imported
Expand Down
95 changes: 63 additions & 32 deletions crates/backend/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1645,15 +1645,42 @@ impl ToTokens for ast::Enum {
impl ToTokens for ast::ImportStatic {
fn to_tokens(&self, into: &mut TokenStream) {
let ty = &self.ty;
static_import(
&self.vis,
&self.rust_name,
&self.wasm_bindgen,
ty,
ty,
&self.shim,
)
.to_tokens(into);

if self.thread_local {
thread_local_import(
&self.vis,
&self.rust_name,
&self.wasm_bindgen,
ty,
ty,
&self.shim,
)
.to_tokens(into)
} else {
let vis = &self.vis;
let name = &self.rust_name;
let wasm_bindgen = &self.wasm_bindgen;
let ty = &self.ty;
let shim_name = &self.shim;
let init = static_init(wasm_bindgen, ty, shim_name);

into.extend(quote! {
#[automatically_derived]
#[deprecated = "use with `#[wasm_bindgen(thread_local)]` instead"]
});
into.extend(
quote_spanned! { name.span() => #vis static #name: #wasm_bindgen::JsStatic<#ty> = {
fn init() -> #ty {
#init
}
thread_local!(static _VAL: #ty = init(););
#wasm_bindgen::JsStatic {
__inner: &_VAL,
}
};
},
);
}

Descriptor {
ident: &self.shim,
Expand All @@ -1672,7 +1699,7 @@ impl ToTokens for ast::ImportString {
let js_sys = &self.js_sys;
let actual_ty: syn::Type = parse_quote!(#js_sys::JsString);

static_import(
thread_local_import(
&self.vis,
&self.rust_name,
&self.wasm_bindgen,
Expand All @@ -1684,41 +1711,45 @@ impl ToTokens for ast::ImportString {
}
}

fn static_import(
fn thread_local_import(
vis: &syn::Visibility,
name: &Ident,
wasm_bindgen: &syn::Path,
actual_ty: &syn::Type,
ty: &syn::Type,
shim_name: &Ident,
) -> TokenStream {
let init = static_init(wasm_bindgen, ty, shim_name);

quote! {
thread_local! {
#[automatically_derived]
#vis static #name: #actual_ty = {
#init
};
}
}
}

fn static_init(wasm_bindgen: &syn::Path, ty: &syn::Type, shim_name: &Ident) -> TokenStream {
let abi_ret = quote! {
#wasm_bindgen::convert::WasmRet<<#ty as #wasm_bindgen::convert::FromWasmAbi>::Abi>
};
quote! {
#[automatically_derived]
#vis static #name: #wasm_bindgen::JsStatic<#actual_ty> = {
fn init() -> #ty {
#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", target_os = "unknown"))]
extern "C" {
fn #shim_name() -> #abi_ret;
}
#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", target_os = "unknown"))]
extern "C" {
fn #shim_name() -> #abi_ret;
}

#[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
unsafe fn #shim_name() -> #abi_ret {
panic!("cannot access imported statics on non-wasm targets")
}
#[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))]
unsafe fn #shim_name() -> #abi_ret {
panic!("cannot access imported statics on non-wasm targets")
}

unsafe {
<#ty as #wasm_bindgen::convert::FromWasmAbi>::from_abi(#shim_name().join())
}
}
thread_local!(static _VAL: #ty = init(););
#wasm_bindgen::JsStatic {
__inner: &_VAL,
}
};
unsafe {
<#ty as #wasm_bindgen::convert::FromWasmAbi>::from_abi(#shim_name().join())
}
}
}

Expand Down
36 changes: 24 additions & 12 deletions crates/js-sys/tests/wasm/Function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ use wasm_bindgen_test::*;

#[wasm_bindgen]
extern "C" {
#[wasm_bindgen(js_name = max, js_namespace = Math)]
#[wasm_bindgen(thread_local, js_name = max, js_namespace = Math)]
static MAX: Function;

type ArrayPrototype;
#[wasm_bindgen(method, getter, structural)]
pub fn push(this: &ArrayPrototype) -> Function;
#[wasm_bindgen(js_name = prototype, js_namespace = Array)]
#[wasm_bindgen(thread_local, js_name = prototype, js_namespace = Array)]
static ARRAY_PROTOTYPE2: ArrayPrototype;
}

Expand All @@ -21,12 +21,19 @@ fn apply() {
args.push(&1.into());
args.push(&2.into());
args.push(&3.into());
assert_eq!(MAX.apply(&JsValue::undefined(), &args).unwrap(), 3);
assert_eq!(
MAX.with(|max| max.apply(&JsValue::undefined(), &args))
.unwrap(),
3
);

let arr = JsValue::from(Array::new());
let args = Array::new();
args.push(&1.into());
ARRAY_PROTOTYPE2.push().apply(&arr, &args).unwrap();
ARRAY_PROTOTYPE2
.with(ArrayPrototype::push)
.apply(&arr, &args)
.unwrap();
assert_eq!(Array::from(&arr).length(), 1);
}

Expand Down Expand Up @@ -111,24 +118,29 @@ fn bind3() {

#[wasm_bindgen_test]
fn length() {
assert_eq!(MAX.length(), 2);
assert_eq!(ARRAY_PROTOTYPE2.push().length(), 1);
assert_eq!(MAX.with(Function::length), 2);
assert_eq!(ARRAY_PROTOTYPE2.with(ArrayPrototype::push).length(), 1);
}

#[wasm_bindgen_test]
fn name() {
assert_eq!(JsValue::from(MAX.name()), "max");
assert_eq!(JsValue::from(ARRAY_PROTOTYPE2.push().name()), "push");
assert_eq!(JsValue::from(MAX.with(Function::name)), "max");
assert_eq!(
JsValue::from(ARRAY_PROTOTYPE2.with(ArrayPrototype::push).name()),
"push"
);
}

#[wasm_bindgen_test]
fn to_string() {
assert!(MAX.to_string().length() > 0);
assert!(MAX.with(Function::to_string).length() > 0);
}

#[wasm_bindgen_test]
fn function_inheritance() {
assert!(MAX.is_instance_of::<Function>());
assert!(MAX.is_instance_of::<Object>());
let _: &Object = MAX.as_ref();
assert!(MAX.with(Function::is_instance_of::<Function>));
assert!(MAX.with(Function::is_instance_of::<Object>));
MAX.with(|max| {
let _: &Object = max.as_ref();
});
}
16 changes: 8 additions & 8 deletions crates/js-sys/tests/wasm/Object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ extern "C" {
#[wasm_bindgen(method, setter, structural)]
fn set_foo(this: &Foo42, val: JsValue);

#[wasm_bindgen(js_name = prototype, js_namespace = Object)]
#[wasm_bindgen(thread_local, js_name = prototype, js_namespace = Object)]
static OBJECT_PROTOTYPE: JsValue;
#[wasm_bindgen(js_name = prototype, js_namespace = Array)]
#[wasm_bindgen(thread_local, js_name = prototype, js_namespace = Array)]
static ARRAY_PROTOTYPE: JsValue;

type DefinePropertyAttrs;
Expand All @@ -32,9 +32,9 @@ extern "C" {
#[wasm_bindgen(constructor)]
fn new() -> Foo;

#[wasm_bindgen(js_name = prototype, js_namespace = Foo)]
#[wasm_bindgen(thread_local, js_name = prototype, js_namespace = Foo)]
static FOO_PROTOTYPE: Object;
#[wasm_bindgen(js_name = prototype, js_namespace = Bar)]
#[wasm_bindgen(thread_local, js_name = prototype, js_namespace = Bar)]
static BAR_PROTOTYPE: Object;
}

Expand Down Expand Up @@ -178,9 +178,9 @@ fn get_own_property_symbols() {
#[wasm_bindgen_test]
fn get_prototype_of() {
let proto = JsValue::from(Object::get_prototype_of(&Object::new().into()));
assert_eq!(proto, *OBJECT_PROTOTYPE);
OBJECT_PROTOTYPE.with(|op| assert_eq!(proto, *op));
let proto = JsValue::from(Object::get_prototype_of(&Array::new().into()));
assert_eq!(proto, *ARRAY_PROTOTYPE);
ARRAY_PROTOTYPE.with(|ap| assert_eq!(proto, *ap));
}

#[wasm_bindgen_test]
Expand Down Expand Up @@ -249,8 +249,8 @@ fn is_sealed() {
#[wasm_bindgen_test]
fn is_prototype_of() {
let foo = JsValue::from(Foo::new());
assert!(FOO_PROTOTYPE.is_prototype_of(&foo));
assert!(!BAR_PROTOTYPE.is_prototype_of(&foo));
assert!(FOO_PROTOTYPE.with(|fp| fp.is_prototype_of(&foo)));
assert!(!BAR_PROTOTYPE.with(|bp| bp.is_prototype_of(&foo)));
}

#[wasm_bindgen_test]
Expand Down
20 changes: 19 additions & 1 deletion crates/macro-support/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ macro_rules! attrgen {
(typescript_type, TypeScriptType(Span, String, Span)),
(getter_with_clone, GetterWithClone(Span)),
(static_string, StaticString(Span)),
(thread_local, ThreadLocal(Span)),

// For testing purposes only.
(assert_no_shim, AssertNoShim(Span)),
Expand Down Expand Up @@ -753,6 +754,13 @@ impl<'a> ConvertToAst<(&ast::Program, BindgenAttrs, &'a Option<ast::ImportModule
bail_span!(self.mutability, "cannot import mutable globals yet")
}

if let Some(span) = opts.static_string() {
return Err(Diagnostic::span_error(
*span,
"static strings require a string literal",
));
}

let default_name = self.ident.to_string();
let js_name = opts
.js_name()
Expand All @@ -764,6 +772,8 @@ impl<'a> ConvertToAst<(&ast::Program, BindgenAttrs, &'a Option<ast::ImportModule
self.ident,
ShortHash((&js_name, module, &self.ident)),
);
let thread_local = opts.thread_local().is_some();

opts.check_used();
Ok(ast::ImportKind::Static(ast::ImportStatic {
ty: *self.ty,
Expand All @@ -772,6 +782,7 @@ impl<'a> ConvertToAst<(&ast::Program, BindgenAttrs, &'a Option<ast::ImportModule
js_name,
shim: Ident::new(&shim, Span::call_site()),
wasm_bindgen: program.wasm_bindgen.clone(),
thread_local,
}))
}
}
Expand Down Expand Up @@ -805,7 +816,14 @@ impl<'a> ConvertToAst<(&ast::Program, BindgenAttrs, &'a Option<ast::ImportModule
if opts.static_string().is_none() {
bail_span!(
self,
"statics strings require `#[wasm_bindgen(static_string)]`"
"static strings require `#[wasm_bindgen(static_string)]`"
)
}

if opts.thread_local().is_none() {
bail_span!(
self,
"static strings require `#[wasm_bindgen(thread_local)]`"
)
}

Expand Down
1 change: 1 addition & 0 deletions crates/macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ wasm-bindgen-macro-support = { path = "../macro-support", version = "=0.2.92" }
quote = "1.0"

[dev-dependencies]
js-sys = { path = "../js-sys" }
trybuild = "1.0"
wasm-bindgen = { path = "../.." }
wasm-bindgen-futures = { path = "../futures" }
Expand Down
15 changes: 13 additions & 2 deletions crates/macro/ui-tests/invalid-items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,26 @@ pub const fn foo2() {}
struct Foo<T>(T);

#[wasm_bindgen]
#[rustfmt::skip]
extern "C" {
static mut FOO: u32;

#[wasm_bindgen(static_string)]
static FOO2: JsString;

#[wasm_bindgen(thread_local, static_string)]
static FOO3: JsString;

static FOO4: JsString = "test";

#[wasm_bindgen(static_string)]
static FOO5: JsString = "test";

pub fn foo3(x: i32, ...);
}

#[wasm_bindgen]
extern "system" {
}
extern "system" {}

#[wasm_bindgen]
pub fn foo4<T>() {}
Expand Down
Loading

0 comments on commit acfd1ee

Please sign in to comment.