Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use reflect in webidl dictionary setters #3898

Merged
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
* Stabilize Web Share API.
[#3882](https://github.com/rustwasm/wasm-bindgen/pull/3882)

* Don't use reflect in webidl dictionary setters
Davidster marked this conversation as resolved.
Show resolved Hide resolved
[#3898](https://github.com/rustwasm/wasm-bindgen/pull/3898)

### Fixed

* Copy port from headless test server when using `WASM_BINDGEN_TEST_ADDRESS`.
Expand Down
8 changes: 4 additions & 4 deletions crates/backend/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ pub struct Operation {
pub enum OperationKind {
/// A standard method, nothing special
Regular,
/// A method for getting the value of the provided Ident
Getter(Option<Ident>),
/// A method for setting the value of the provided Ident
Setter(Option<Ident>),
/// A method for getting the value of the provided Ident or String
Getter(Option<String>),
/// A method for setting the value of the provided Ident or String
Setter(Option<String>),
/// A dynamically intercepted getter
IndexingGetter,
/// A dynamically intercepted setter
Expand Down
4 changes: 2 additions & 2 deletions crates/backend/src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,12 +531,12 @@ fn from_ast_method_kind<'a>(
let is_static = *is_static;
let kind = match kind {
ast::OperationKind::Getter(g) => {
let g = g.as_ref().map(|g| intern.intern(g));
let g = g.as_ref().map(|g| intern.intern_str(g));
OperationKind::Getter(g.unwrap_or_else(|| function.infer_getter_property()))
}
ast::OperationKind::Regular => OperationKind::Regular,
ast::OperationKind::Setter(s) => {
let s = s.as_ref().map(|s| intern.intern(s));
let s = s.as_ref().map(|s| intern.intern_str(s));
OperationKind::Setter(match s {
Some(s) => s,
None => intern.intern_str(&function.infer_setter_property()?),
Expand Down
13 changes: 9 additions & 4 deletions crates/macro-support/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ macro_rules! attrgen {
(module, Module(Span, String, Span)),
(raw_module, RawModule(Span, String, Span)),
(inline_js, InlineJs(Span, String, Span)),
(getter, Getter(Span, Option<Ident>)),
(setter, Setter(Span, Option<Ident>)),
(getter, Getter(Span, Option<String>)),
(setter, Setter(Span, Option<String>)),
(indexing_getter, IndexingGetter(Span)),
(indexing_setter, IndexingSetter(Span)),
(indexing_deleter, IndexingDeleter(Span)),
Expand Down Expand Up @@ -299,10 +299,15 @@ impl Parse for BindgenAttr {
return Ok(BindgenAttr::$variant(attr_span, ident))
});

(@parser $variant:ident(Span, Option<Ident>)) => ({
(@parser $variant:ident(Span, Option<String>)) => ({
if input.parse::<Token![=]>().is_ok() {
if input.peek(syn::LitStr) {
let litstr = input.parse::<syn::LitStr>()?;
return Ok(BindgenAttr::$variant(attr_span, Some(litstr.value())))
}

let ident = input.parse::<AnyIdent>()?.0;
return Ok(BindgenAttr::$variant(attr_span, Some(ident)))
return Ok(BindgenAttr::$variant(attr_span, Some(ident.to_string())))
} else {
return Ok(BindgenAttr::$variant(attr_span, None));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/webidl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ heck = "0.3"
log = "0.4.1"
proc-macro2 = "1.0"
quote = '1.0'
syn = { version = '2.0', features = ['full'] }
syn = { version = '2.0', features = ['full', 'extra-traits'] }
daxpedda marked this conversation as resolved.
Show resolved Hide resolved
wasm-bindgen-backend = { version = "=0.2.92", path = "../backend" }
weedle = { git = "https://github.com/rustwasm/weedle.git" }
once_cell = "1.12"
Expand Down
115 changes: 94 additions & 21 deletions crates/webidl/src/generator.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
use proc_macro2::Literal;
use proc_macro2::TokenStream;
use quote::format_ident;
use quote::quote;
use std::collections::BTreeSet;
use syn::{Ident, Type};
use wasm_bindgen_backend::util::leading_colon_path_ty;
use wasm_bindgen_backend::util::{raw_ident, rust_ident};

use crate::constants::{BUILTIN_IDENTS, POLYFILL_INTERFACES};
use crate::traverse::TraverseType;
use crate::util::option_ty;
use crate::util::shared_ref;
use crate::util::{get_cfg_features, mdn_doc, required_doc_string, snake_case_ident};
use crate::Options;

Expand Down Expand Up @@ -630,7 +634,40 @@ pub struct DictionaryField {
}

impl DictionaryField {
fn generate_rust(&self, options: &Options, parent_name: String) -> TokenStream {
fn generate_rust_shim(
&self,
parent_ident: &Ident,
cfg_features: &Option<syn::Attribute>,
) -> TokenStream {
let ty = &self.ty;
let shim_name = self.shim_name();
let js_name = &self.js_name;

let js_value_ref_type = shared_ref(
leading_colon_path_ty(vec![rust_ident("wasm_bindgen"), rust_ident("JsValue")]),
false,
);
let js_value_ref_option_type = option_ty(js_value_ref_type.clone());

let ty = if ty == &js_value_ref_option_type {
js_value_ref_type
} else {
ty.clone()
};

quote! {
#cfg_features
#[wasm_bindgen(method, setter = #js_name)]
fn #shim_name(this: &#parent_ident, val: #ty);
}
}

fn generate_rust_setter(
&self,
options: &Options,
features: &BTreeSet<String>,
cfg_features: &Option<syn::Attribute>,
) -> TokenStream {
let DictionaryField {
name,
js_name,
Expand All @@ -642,39 +679,58 @@ impl DictionaryField {
let unstable_attr = maybe_unstable_attr(*unstable);
let unstable_docs = maybe_unstable_docs(*unstable);

let mut features = BTreeSet::new();

add_features(&mut features, ty);

features.remove(&parent_name);

let cfg_features = get_cfg_features(options, &features);

features.insert(parent_name);

let doc_comment = comment(
format!("Change the `{}` field of this object.", js_name),
&required_doc_string(options, &features),
&required_doc_string(options, features),
);

let shim_name = self.shim_name();

let js_value_ref_type = shared_ref(
leading_colon_path_ty(vec![rust_ident("wasm_bindgen"), rust_ident("JsValue")]),
false,
);
let js_value_ref_option_type = option_ty(js_value_ref_type.clone());

let shim_args = if ty == &js_value_ref_option_type {
quote! { val.unwrap_or(#js_value_ref_type::NULL) }
Davidster marked this conversation as resolved.
Show resolved Hide resolved
} else {
quote! { val }
};

quote! {
#unstable_attr
#cfg_features
#doc_comment
#unstable_docs
pub fn #name(&mut self, val: #ty) -> &mut Self {
use wasm_bindgen::JsValue;
let r = ::js_sys::Reflect::set(
self.as_ref(),
&JsValue::from(#js_name),
&JsValue::from(val),
);
debug_assert!(r.is_ok(), "setting properties should never fail on our dictionary objects");
let _ = r;
self.#shim_name(#shim_args);
self
}
}
}

fn features(
&self,
options: &Options,
parent_name: String,
) -> (BTreeSet<String>, Option<syn::Attribute>) {
let mut features = BTreeSet::new();

add_features(&mut features, &self.ty);

features.remove(&parent_name);

let cfg_features = get_cfg_features(options, &features);

features.insert(parent_name);

(features, cfg_features)
}

fn shim_name(&self) -> Ident {
format_ident!("{}_shim", self.name)
}
}

pub struct Dictionary {
Expand Down Expand Up @@ -727,9 +783,24 @@ impl Dictionary {
&required_doc_string(options, &required_features),
);

let (field_features, field_cfg_features): (Vec<_>, Vec<_>) = fields
.iter()
.map(|field| field.features(options, name.to_string()))
.unzip();

let field_shims = fields
.iter()
.zip(field_cfg_features.iter())
.map(|(field, cfg_features)| field.generate_rust_shim(name, cfg_features))
.collect::<Vec<_>>();

let fields = fields
.iter()
.map(|field| field.generate_rust(options, name.to_string()))
.zip(field_features.iter())
.zip(field_cfg_features.iter())
.map(|((field, features), cfg_features)| {
field.generate_rust_setter(options, features, cfg_features)
})
.collect::<Vec<_>>();

let mut base_stream = quote! {
Expand All @@ -746,6 +817,8 @@ impl Dictionary {
#doc_comment
#unstable_docs
pub type #name;

#(#field_shims)*
}

#unstable_attr
Expand Down
Loading