From 75f7214d93968d57bad7a575bfaf856c57b01b90 Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Mon, 30 Mar 2020 13:24:02 +0800 Subject: [PATCH 01/10] use global import map for rename --- crates/cli-support/src/js/mod.rs | 80 ++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 14 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index fd0f1e11c55..359828f79cf 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -42,6 +42,7 @@ pub struct Context<'a> { /// the number of times they've been used, used to generate new /// identifiers. defined_identifiers: HashMap, + global_defined_import_identifiers: HashMap, exported_classes: Option>, @@ -90,6 +91,7 @@ impl<'a> Context<'a> { imported_names: Default::default(), js_imports: Default::default(), defined_identifiers: Default::default(), + global_defined_import_identifiers: Default::default(), wasm_import_definitions: Default::default(), exported_classes: Some(Default::default()), config, @@ -113,7 +115,7 @@ impl<'a> Context<'a> { contents: &str, comments: Option<&str>, ) -> Result<(), Error> { - let definition_name = generate_identifier(export_name, &mut self.defined_identifiers); + let definition_name = generate_identifier(export_name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); if contents.starts_with("class") && definition_name != export_name { bail!("cannot shadow already defined class `{}`", export_name); } @@ -1945,13 +1947,13 @@ impl<'a> Context<'a> { let mut name = match &import.name { JsImportName::Module { module, name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers); + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); add_module_import(module.clone(), name, &unique_name); unique_name } JsImportName::LocalModule { module, name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers); + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); let module = self.config.local_module_name(module); add_module_import(module, name, &unique_name); unique_name @@ -1965,7 +1967,7 @@ impl<'a> Context<'a> { let module = self .config .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); - let unique_name = generate_identifier(name, &mut self.defined_identifiers); + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); add_module_import(module, name, &unique_name); unique_name } @@ -1996,11 +1998,7 @@ impl<'a> Context<'a> { } JsImportName::Global { name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers); - if unique_name != *name { - bail!("cannot import `{}` from two locations", name); - } - unique_name + if name == "default" { format!("{}{}", name, 1) } else { name.clone() } } }; self.imported_names @@ -2053,6 +2051,7 @@ impl<'a> Context<'a> { } pub fn generate(&mut self) -> Result<(), Error> { + self.precache_global_import_identifiers()?; for (id, adapter) in crate::sorted_iter(&self.wit.adapters) { let instrs = match &adapter.kind { AdapterKind::Import { .. } => continue, @@ -2082,6 +2081,53 @@ impl<'a> Context<'a> { Ok(()) } + fn precache_global_import_identifiers(&mut self) -> Result<(), Error> { + for (_id, adapter) in crate::sorted_iter(&self.wit.adapters) { + let instrs = match &adapter.kind { + AdapterKind::Import { .. } => continue, + AdapterKind::Local { instructions } => instructions, + }; + let mut call = None; + for instr in instrs { + match instr.instr { + Instruction::CallAdapter(id) => { + if call.is_some() { + continue + } else { + call = Some(id); + } + } + Instruction::CallExport(_) + | Instruction::CallTableElement(_) + | Instruction::Standard(wit_walrus::Instruction::CallCore(_)) + | Instruction::Standard(wit_walrus::Instruction::CallAdapter(_)) => { + continue + } + _ => {} + } + } + if let Some(id) = call { + let js = match &self.aux.import_map[&id] { + AuxImport::Value(AuxValue::Bare(js)) => Some(js), + _ => None, + }; + if let Some(js) = js { + match &js.name { + JsImportName::Global { name } => { + let cnt = self.global_defined_import_identifiers.entry(name.clone()).or_insert(0); + if *cnt == 1 { + bail!("cannot import `{}` from two locations", name); + } + *cnt += 1; + } + _ => {} + } + } + } + } + Ok(()) + } + fn generate_adapter( &mut self, id: AdapterId, @@ -3165,9 +3211,14 @@ fn check_duplicated_getter_and_setter_names( Ok(()) } -fn generate_identifier(name: &str, used_names: &mut HashMap) -> String { +fn generate_identifier(name: &str, used_names: &mut HashMap, global_names: &mut HashMap) -> String { let cnt = used_names.entry(name.to_string()).or_insert(0); *cnt += 1; + let global_cnt = (*global_names.entry(name.to_string()).or_insert(0)).clone(); + // leave index for global import + if *cnt == 1 && global_cnt != 0 { + *cnt += 1; + } // We want to mangle `default` at once, so we can support default exports and don't generate // invalid glue code like this: `import { default } from './module';`. if *cnt == 1 && name != "default" { @@ -3272,20 +3323,21 @@ impl ExportedClass { #[test] fn test_generate_identifier() { let mut used_names: HashMap = HashMap::new(); + let mut global_used_names: HashMap = HashMap::new(); assert_eq!( - generate_identifier("someVar", &mut used_names), + generate_identifier("someVar", &mut used_names, &mut global_used_names), "someVar".to_string() ); assert_eq!( - generate_identifier("someVar", &mut used_names), + generate_identifier("someVar", &mut used_names, &mut global_used_names), "someVar2".to_string() ); assert_eq!( - generate_identifier("default", &mut used_names), + generate_identifier("default", &mut used_names, &mut global_used_names), "default1".to_string() ); assert_eq!( - generate_identifier("default", &mut used_names), + generate_identifier("default", &mut used_names, &mut global_used_names), "default2".to_string() ); } From 94321b8d4ed3b6e0b64ad3c009c7efb2e6b184ba Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Mon, 30 Mar 2020 22:40:15 +0800 Subject: [PATCH 02/10] fix same ns import --- crates/cli-support/src/js/mod.rs | 44 +++++++++++++++++--------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 359828f79cf..8e7a50dad57 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -115,7 +115,7 @@ impl<'a> Context<'a> { contents: &str, comments: Option<&str>, ) -> Result<(), Error> { - let definition_name = generate_identifier(export_name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); + let definition_name = generate_identifier(export_name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); if contents.starts_with("class") && definition_name != export_name { bail!("cannot shadow already defined class `{}`", export_name); } @@ -1947,13 +1947,13 @@ impl<'a> Context<'a> { let mut name = match &import.name { JsImportName::Module { module, name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); add_module_import(module.clone(), name, &unique_name); unique_name } JsImportName::LocalModule { module, name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); let module = self.config.local_module_name(module); add_module_import(module, name, &unique_name); unique_name @@ -1967,7 +1967,7 @@ impl<'a> Context<'a> { let module = self .config .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers); + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); add_module_import(module, name, &unique_name); unique_name } @@ -1998,7 +1998,11 @@ impl<'a> Context<'a> { } JsImportName::Global { name } => { - if name == "default" { format!("{}{}", name, 1) } else { name.clone() } + let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, true); + if unique_name != *name { + bail!("cannot import `{}` from two locations", name); + } + unique_name } }; self.imported_names @@ -2114,11 +2118,7 @@ impl<'a> Context<'a> { if let Some(js) = js { match &js.name { JsImportName::Global { name } => { - let cnt = self.global_defined_import_identifiers.entry(name.clone()).or_insert(0); - if *cnt == 1 { - bail!("cannot import `{}` from two locations", name); - } - *cnt += 1; + self.global_defined_import_identifiers.entry(name.clone()).or_insert(1); } _ => {} } @@ -3211,17 +3211,21 @@ fn check_duplicated_getter_and_setter_names( Ok(()) } -fn generate_identifier(name: &str, used_names: &mut HashMap, global_names: &mut HashMap) -> String { +fn generate_identifier(name: &str, used_names: &mut HashMap, global_names: &mut HashMap, global_import: bool) -> String { let cnt = used_names.entry(name.to_string()).or_insert(0); - *cnt += 1; - let global_cnt = (*global_names.entry(name.to_string()).or_insert(0)).clone(); - // leave index for global import - if *cnt == 1 && global_cnt != 0 { + let global_cnt = global_names.entry(name.to_string()).or_insert(0); + if global_import { + *global_cnt += 1; + } else { *cnt += 1; + // leave index for global import + if *cnt == 1 && *global_cnt != 0 { + *cnt += 1; + } } // We want to mangle `default` at once, so we can support default exports and don't generate // invalid glue code like this: `import { default } from './module';`. - if *cnt == 1 && name != "default" { + if ((!global_import && *cnt == 1) || (global_import && *global_cnt <= 2)) && name != "default" { name.to_string() } else { format!("{}{}", name, cnt) @@ -3325,19 +3329,19 @@ fn test_generate_identifier() { let mut used_names: HashMap = HashMap::new(); let mut global_used_names: HashMap = HashMap::new(); assert_eq!( - generate_identifier("someVar", &mut used_names, &mut global_used_names), + generate_identifier("someVar", &mut used_names, &mut global_used_names, false), "someVar".to_string() ); assert_eq!( - generate_identifier("someVar", &mut used_names, &mut global_used_names), + generate_identifier("someVar", &mut used_names, &mut global_used_names, false), "someVar2".to_string() ); assert_eq!( - generate_identifier("default", &mut used_names, &mut global_used_names), + generate_identifier("default", &mut used_names, &mut global_used_names, false), "default1".to_string() ); assert_eq!( - generate_identifier("default", &mut used_names, &mut global_used_names), + generate_identifier("default", &mut used_names, &mut global_used_names, false), "default2".to_string() ); } From 0716a2500eef5e83edf1c821afa1e57352a8689d Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Mon, 30 Mar 2020 22:53:10 +0800 Subject: [PATCH 03/10] cargo fmt --- crates/cli-support/src/js/mod.rs | 52 +++++++++++++++++++++++++------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 8e7a50dad57..f26e1afcc5c 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -115,7 +115,12 @@ impl<'a> Context<'a> { contents: &str, comments: Option<&str>, ) -> Result<(), Error> { - let definition_name = generate_identifier(export_name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); + let definition_name = generate_identifier( + export_name, + &mut self.defined_identifiers, + &mut self.global_defined_import_identifiers, + false, + ); if contents.starts_with("class") && definition_name != export_name { bail!("cannot shadow already defined class `{}`", export_name); } @@ -1947,13 +1952,23 @@ impl<'a> Context<'a> { let mut name = match &import.name { JsImportName::Module { module, name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); + let unique_name = generate_identifier( + name, + &mut self.defined_identifiers, + &mut self.global_defined_import_identifiers, + false, + ); add_module_import(module.clone(), name, &unique_name); unique_name } JsImportName::LocalModule { module, name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); + let unique_name = generate_identifier( + name, + &mut self.defined_identifiers, + &mut self.global_defined_import_identifiers, + false, + ); let module = self.config.local_module_name(module); add_module_import(module, name, &unique_name); unique_name @@ -1967,7 +1982,12 @@ impl<'a> Context<'a> { let module = self .config .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, false); + let unique_name = generate_identifier( + name, + &mut self.defined_identifiers, + &mut self.global_defined_import_identifiers, + false, + ); add_module_import(module, name, &unique_name); unique_name } @@ -1998,7 +2018,12 @@ impl<'a> Context<'a> { } JsImportName::Global { name } => { - let unique_name = generate_identifier(name, &mut self.defined_identifiers, &mut self.global_defined_import_identifiers, true); + let unique_name = generate_identifier( + name, + &mut self.defined_identifiers, + &mut self.global_defined_import_identifiers, + true, + ); if unique_name != *name { bail!("cannot import `{}` from two locations", name); } @@ -2096,7 +2121,7 @@ impl<'a> Context<'a> { match instr.instr { Instruction::CallAdapter(id) => { if call.is_some() { - continue + continue; } else { call = Some(id); } @@ -2104,9 +2129,7 @@ impl<'a> Context<'a> { Instruction::CallExport(_) | Instruction::CallTableElement(_) | Instruction::Standard(wit_walrus::Instruction::CallCore(_)) - | Instruction::Standard(wit_walrus::Instruction::CallAdapter(_)) => { - continue - } + | Instruction::Standard(wit_walrus::Instruction::CallAdapter(_)) => continue, _ => {} } } @@ -2118,7 +2141,9 @@ impl<'a> Context<'a> { if let Some(js) = js { match &js.name { JsImportName::Global { name } => { - self.global_defined_import_identifiers.entry(name.clone()).or_insert(1); + self.global_defined_import_identifiers + .entry(name.clone()) + .or_insert(1); } _ => {} } @@ -3211,7 +3236,12 @@ fn check_duplicated_getter_and_setter_names( Ok(()) } -fn generate_identifier(name: &str, used_names: &mut HashMap, global_names: &mut HashMap, global_import: bool) -> String { +fn generate_identifier( + name: &str, + used_names: &mut HashMap, + global_names: &mut HashMap, + global_import: bool, +) -> String { let cnt = used_names.entry(name.to_string()).or_insert(0); let global_cnt = global_names.entry(name.to_string()).or_insert(0); if global_import { From 72a19e0d08250654fd72a8cc8d0786160ec290f9 Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Mon, 30 Mar 2020 23:12:47 +0800 Subject: [PATCH 04/10] add basic test --- crates/cli/tests/wasm-bindgen/main.rs | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/cli/tests/wasm-bindgen/main.rs b/crates/cli/tests/wasm-bindgen/main.rs index a65b89a0d9e..a2846a064cd 100644 --- a/crates/cli/tests/wasm-bindgen/main.rs +++ b/crates/cli/tests/wasm-bindgen/main.rs @@ -135,6 +135,34 @@ fn works_on_empty_project() { cmd.assert().success(); } +#[test] +fn namespace_global_and_noglobal_works() { + let (mut cmd, _out_dir) = Project::new("namespace_global_and_noglobal_works") + .file( + "src/lib.rs", + r#" + use wasm_bindgen::prelude::*; + #[wasm_bindgen(module = "fs")] + extern "C" { + #[wasm_bindgen(js_namespace = window)] + fn t1(); + } + #[wasm_bindgen] + extern "C" { + #[wasm_bindgen(js_namespace = window)] + fn t2(); + } + #[wasm_bindgen] + pub fn test() { + t1(); + t2(); + } + "#, + ) + .wasm_bindgen(""); + cmd.assert().success(); +} + mod npm; #[test] From 2eeff656a941030222b298f79c1b811ea702697a Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Sat, 11 Apr 2020 17:09:14 +0800 Subject: [PATCH 05/10] move generate_identifier, add comments, add tests --- crates/cli-support/src/js/mod.rs | 149 +++++++++++-------------------- tests/wasm/imports.js | 10 +++ tests/wasm/imports.rs | 39 ++++++++ tests/wasm/imports_2.js | 5 ++ 4 files changed, 107 insertions(+), 96 deletions(-) create mode 100644 tests/wasm/imports_2.js diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index f26e1afcc5c..8ad7fa27cb1 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -115,12 +115,7 @@ impl<'a> Context<'a> { contents: &str, comments: Option<&str>, ) -> Result<(), Error> { - let definition_name = generate_identifier( - export_name, - &mut self.defined_identifiers, - &mut self.global_defined_import_identifiers, - false, - ); + let definition_name = self.generate_identifier(export_name, false); if contents.starts_with("class") && definition_name != export_name { bail!("cannot shadow already defined class `{}`", export_name); } @@ -1927,6 +1922,18 @@ impl<'a> Context<'a> { require_class(&mut self.exported_classes, name).wrap_needed = true; } + fn add_module_import(&mut self, module: String, name: &str, actual: &str) { + let rename = if name == actual { + None + } else { + Some(actual.to_string()) + }; + (&mut self.js_imports) + .entry(module) + .or_insert(Vec::new()) + .push((name.to_string(), rename)); + } + fn import_name(&mut self, import: &JsImport) -> Result { if let Some(name) = self.imported_names.get(&import.name) { let mut name = name.clone(); @@ -1937,40 +1944,17 @@ impl<'a> Context<'a> { return Ok(name.clone()); } - let js_imports = &mut self.js_imports; - let mut add_module_import = |module: String, name: &str, actual: &str| { - let rename = if name == actual { - None - } else { - Some(actual.to_string()) - }; - js_imports - .entry(module) - .or_insert(Vec::new()) - .push((name.to_string(), rename)); - }; - let mut name = match &import.name { JsImportName::Module { module, name } => { - let unique_name = generate_identifier( - name, - &mut self.defined_identifiers, - &mut self.global_defined_import_identifiers, - false, - ); - add_module_import(module.clone(), name, &unique_name); + let unique_name = self.generate_identifier(name, false); + self.add_module_import(module.clone(), name, &unique_name); unique_name } JsImportName::LocalModule { module, name } => { - let unique_name = generate_identifier( - name, - &mut self.defined_identifiers, - &mut self.global_defined_import_identifiers, - false, - ); + let unique_name = self.generate_identifier(name, false); let module = self.config.local_module_name(module); - add_module_import(module, name, &unique_name); + self.add_module_import(module, name, &unique_name); unique_name } @@ -1982,13 +1966,8 @@ impl<'a> Context<'a> { let module = self .config .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); - let unique_name = generate_identifier( - name, - &mut self.defined_identifiers, - &mut self.global_defined_import_identifiers, - false, - ); - add_module_import(module, name, &unique_name); + let unique_name = self.generate_identifier(name, false); + self.add_module_import(module, name, &unique_name); unique_name } @@ -2018,12 +1997,7 @@ impl<'a> Context<'a> { } JsImportName::Global { name } => { - let unique_name = generate_identifier( - name, - &mut self.defined_identifiers, - &mut self.global_defined_import_identifiers, - true, - ); + let unique_name = self.generate_identifier(name, true); if unique_name != *name { bail!("cannot import `{}` from two locations", name); } @@ -2080,7 +2054,7 @@ impl<'a> Context<'a> { } pub fn generate(&mut self) -> Result<(), Error> { - self.precache_global_import_identifiers()?; + self.prestore_global_import_identifiers()?; for (id, adapter) in crate::sorted_iter(&self.wit.adapters) { let instrs = match &adapter.kind { AdapterKind::Import { .. } => continue, @@ -2110,7 +2084,7 @@ impl<'a> Context<'a> { Ok(()) } - fn precache_global_import_identifiers(&mut self) -> Result<(), Error> { + fn prestore_global_import_identifiers(&mut self) -> Result<(), Error> { for (_id, adapter) in crate::sorted_iter(&self.wit.adapters) { let instrs = match &adapter.kind { AdapterKind::Import { .. } => continue, @@ -3189,6 +3163,37 @@ impl<'a> Context<'a> { fn adapter_name(&self, id: AdapterId) -> String { format!("__wbg_adapter_{}", id.0) } + + fn generate_identifier(&mut self, name: &str, global_import: bool) -> String { + let cnt = (&mut self.defined_identifiers) + .entry(name.to_string()) + .or_insert(0); + let global_cnt = (&mut self.global_defined_import_identifiers) + .entry(name.to_string()) + .or_insert(0); + if global_import { + *global_cnt += 1; + } else { + *cnt += 1; + // in this case, we encounter a non-global import name, which conflicts with a pre-stored global one, + // for the first time (*cnt == 1). So leave space for global name and start with `some_import_name_1` + if *cnt == 1 && *global_cnt != 0 { + *cnt += 1; + } + } + // We want to mangle `default` at once, so we can support default exports and don't generate + // invalid glue code like this: `import { default } from './module';`. + // for non-global import, rename is required when `*cnt > 1` + // for global import, since we use `*cnt = 1` for pre-store before entering this function, + // rename is required when `*cnt > 2` + if ((!global_import && *cnt == 1) || (global_import && *global_cnt <= 2)) + && name != "default" + { + name.to_string() + } else { + format!("{}{}", name, cnt) + } + } } fn check_duplicated_getter_and_setter_names( @@ -3236,32 +3241,6 @@ fn check_duplicated_getter_and_setter_names( Ok(()) } -fn generate_identifier( - name: &str, - used_names: &mut HashMap, - global_names: &mut HashMap, - global_import: bool, -) -> String { - let cnt = used_names.entry(name.to_string()).or_insert(0); - let global_cnt = global_names.entry(name.to_string()).or_insert(0); - if global_import { - *global_cnt += 1; - } else { - *cnt += 1; - // leave index for global import - if *cnt == 1 && *global_cnt != 0 { - *cnt += 1; - } - } - // We want to mangle `default` at once, so we can support default exports and don't generate - // invalid glue code like this: `import { default } from './module';`. - if ((!global_import && *cnt == 1) || (global_import && *global_cnt <= 2)) && name != "default" { - name.to_string() - } else { - format!("{}{}", name, cnt) - } -} - fn format_doc_comments(comments: &str, js_doc_comments: Option) -> String { let body: String = comments.lines().map(|c| format!("*{}\n", c)).collect(); let doc = if let Some(docs) = js_doc_comments { @@ -3354,28 +3333,6 @@ impl ExportedClass { } } -#[test] -fn test_generate_identifier() { - let mut used_names: HashMap = HashMap::new(); - let mut global_used_names: HashMap = HashMap::new(); - assert_eq!( - generate_identifier("someVar", &mut used_names, &mut global_used_names, false), - "someVar".to_string() - ); - assert_eq!( - generate_identifier("someVar", &mut used_names, &mut global_used_names, false), - "someVar2".to_string() - ); - assert_eq!( - generate_identifier("default", &mut used_names, &mut global_used_names, false), - "default1".to_string() - ); - assert_eq!( - generate_identifier("default", &mut used_names, &mut global_used_names, false), - "default2".to_string() - ); -} - struct MemView { name: &'static str, num: usize, diff --git a/tests/wasm/imports.js b/tests/wasm/imports.js index 44182a38645..d8bf4ed0f67 100644 --- a/tests/wasm/imports.js +++ b/tests/wasm/imports.js @@ -127,3 +127,13 @@ exports.receive_some = val => { }; exports.get_some_val = () => VAL; + +exports.Math = { + func_from_module_math: (a) => a * 2 +} + +exports.same_name_from_import = (a) => a * 3; + +exports.same_js_namespace_from_module = { + func_from_module_1_same_js_namespace: (a) => a * 5 +} \ No newline at end of file diff --git a/tests/wasm/imports.rs b/tests/wasm/imports.rs index 116c7aff8d4..de7438d153e 100644 --- a/tests/wasm/imports.rs +++ b/tests/wasm/imports.rs @@ -69,11 +69,32 @@ extern "C" { fn receive_some_ref(arg: Option<&PassOutOptionUndefined>); #[wasm_bindgen(js_name = "receive_some")] fn receive_some_owned(arg: Option); + + #[wasm_bindgen(js_namespace = Math)] + fn func_from_module_math(a: i32) -> i32; + + #[wasm_bindgen(js_name = "same_name_from_import")] + fn same_name_from_import_1(s: i32) -> i32; + + #[wasm_bindgen(js_namespace = same_js_namespace_from_module)] + fn func_from_module_1_same_js_namespace(s: i32) -> i32; +} + +#[wasm_bindgen(module = "tests/wasm/imports_2.js")] +extern "C" { + #[wasm_bindgen(js_name = "same_name_from_import")] + fn same_name_from_import_2(s: i32) -> i32; + + #[wasm_bindgen(js_namespace = same_js_namespace_from_module)] + fn func_from_module_2_same_js_namespace(s: i32) -> i32; } #[wasm_bindgen] extern "C" { fn parseInt(a: &str) -> u32; + + #[wasm_bindgen(js_namespace = Math, js_name = "sqrt")] + fn func_from_global_math(s: f64) -> f64; } #[wasm_bindgen_test] @@ -274,3 +295,21 @@ fn pass_out_options_as_undefined() { receive_some_owned(Some(v.clone())); receive_some_owned(Some(v)); } + +#[wasm_bindgen_test] +fn func_from_global_and_module_same_js_namespace() { + assert_eq!(func_from_global_math(4.0), 2.0); + assert_eq!(func_from_module_math(2), 4); +} + +#[wasm_bindgen_test] +fn func_from_two_modules_same_js_name() { + assert_eq!(same_name_from_import_1(1), 3); + assert_eq!(same_name_from_import_2(1), 4); +} + +#[wasm_bindgen_test] +fn func_from_two_modules_same_js_namespace() { + assert_eq!(func_from_module_1_same_js_namespace(2), 10); + assert_eq!(func_from_module_2_same_js_namespace(2), 12); +} diff --git a/tests/wasm/imports_2.js b/tests/wasm/imports_2.js new file mode 100644 index 00000000000..78f55be0fcf --- /dev/null +++ b/tests/wasm/imports_2.js @@ -0,0 +1,5 @@ +exports.same_name_from_import = (a) => a * 4; + +exports.same_js_namespace_from_module = { + func_from_module_2_same_js_namespace: (a) => a * 6 +} \ No newline at end of file From e43f049459de14295027079359d23766c1d27fc6 Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Mon, 13 Apr 2020 23:21:14 +0800 Subject: [PATCH 06/10] remove leading &mut --- crates/cli-support/src/js/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 8ad7fa27cb1..7927637ab21 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -1928,7 +1928,7 @@ impl<'a> Context<'a> { } else { Some(actual.to_string()) }; - (&mut self.js_imports) + self.js_imports .entry(module) .or_insert(Vec::new()) .push((name.to_string(), rename)); @@ -3165,7 +3165,8 @@ impl<'a> Context<'a> { } fn generate_identifier(&mut self, name: &str, global_import: bool) -> String { - let cnt = (&mut self.defined_identifiers) + let cnt = self + .defined_identifiers .entry(name.to_string()) .or_insert(0); let global_cnt = (&mut self.global_defined_import_identifiers) From 511a5839ecedce5c8557f5b57798758791c13638 Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Tue, 14 Apr 2020 19:57:03 +0800 Subject: [PATCH 07/10] remove unnecessary bail --- crates/cli-support/src/js/mod.rs | 45 +++++++------------------------- 1 file changed, 9 insertions(+), 36 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 7927637ab21..12f749dca70 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -42,7 +42,6 @@ pub struct Context<'a> { /// the number of times they've been used, used to generate new /// identifiers. defined_identifiers: HashMap, - global_defined_import_identifiers: HashMap, exported_classes: Option>, @@ -91,7 +90,6 @@ impl<'a> Context<'a> { imported_names: Default::default(), js_imports: Default::default(), defined_identifiers: Default::default(), - global_defined_import_identifiers: Default::default(), wasm_import_definitions: Default::default(), exported_classes: Some(Default::default()), config, @@ -115,7 +113,7 @@ impl<'a> Context<'a> { contents: &str, comments: Option<&str>, ) -> Result<(), Error> { - let definition_name = self.generate_identifier(export_name, false); + let definition_name = self.generate_identifier(export_name); if contents.starts_with("class") && definition_name != export_name { bail!("cannot shadow already defined class `{}`", export_name); } @@ -1946,13 +1944,13 @@ impl<'a> Context<'a> { let mut name = match &import.name { JsImportName::Module { module, name } => { - let unique_name = self.generate_identifier(name, false); + let unique_name = self.generate_identifier(name); self.add_module_import(module.clone(), name, &unique_name); unique_name } JsImportName::LocalModule { module, name } => { - let unique_name = self.generate_identifier(name, false); + let unique_name = self.generate_identifier(name); let module = self.config.local_module_name(module); self.add_module_import(module, name, &unique_name); unique_name @@ -1966,7 +1964,7 @@ impl<'a> Context<'a> { let module = self .config .inline_js_module_name(unique_crate_identifier, *snippet_idx_in_crate); - let unique_name = self.generate_identifier(name, false); + let unique_name = self.generate_identifier(name); self.add_module_import(module, name, &unique_name); unique_name } @@ -1996,13 +1994,7 @@ impl<'a> Context<'a> { format!("l{}", name) } - JsImportName::Global { name } => { - let unique_name = self.generate_identifier(name, true); - if unique_name != *name { - bail!("cannot import `{}` from two locations", name); - } - unique_name - } + JsImportName::Global { name } => name.to_string(), }; self.imported_names .insert(import.name.clone(), name.clone()); @@ -2115,9 +2107,7 @@ impl<'a> Context<'a> { if let Some(js) = js { match &js.name { JsImportName::Global { name } => { - self.global_defined_import_identifiers - .entry(name.clone()) - .or_insert(1); + self.generate_identifier(name); } _ => {} } @@ -3164,32 +3154,15 @@ impl<'a> Context<'a> { format!("__wbg_adapter_{}", id.0) } - fn generate_identifier(&mut self, name: &str, global_import: bool) -> String { + fn generate_identifier(&mut self, name: &str) -> String { let cnt = self .defined_identifiers .entry(name.to_string()) .or_insert(0); - let global_cnt = (&mut self.global_defined_import_identifiers) - .entry(name.to_string()) - .or_insert(0); - if global_import { - *global_cnt += 1; - } else { - *cnt += 1; - // in this case, we encounter a non-global import name, which conflicts with a pre-stored global one, - // for the first time (*cnt == 1). So leave space for global name and start with `some_import_name_1` - if *cnt == 1 && *global_cnt != 0 { - *cnt += 1; - } - } + *cnt += 1; // We want to mangle `default` at once, so we can support default exports and don't generate // invalid glue code like this: `import { default } from './module';`. - // for non-global import, rename is required when `*cnt > 1` - // for global import, since we use `*cnt = 1` for pre-store before entering this function, - // rename is required when `*cnt > 2` - if ((!global_import && *cnt == 1) || (global_import && *global_cnt <= 2)) - && name != "default" - { + if *cnt == 1 && name != "default" { name.to_string() } else { format!("{}{}", name, cnt) From 1b92ea8caa1e5cdb51a7a9ec088875a5b7cf6e66 Mon Sep 17 00:00:00 2001 From: a1trl9 Date: Wed, 15 Apr 2020 09:51:06 +0800 Subject: [PATCH 08/10] use import_name for global and some refine --- crates/cli-support/src/js/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 12f749dca70..959202a96c1 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -1994,7 +1994,7 @@ impl<'a> Context<'a> { format!("l{}", name) } - JsImportName::Global { name } => name.to_string(), + JsImportName::Global { name } => self.generate_identifier(name), }; self.imported_names .insert(import.name.clone(), name.clone()); @@ -2106,8 +2106,8 @@ impl<'a> Context<'a> { }; if let Some(js) = js { match &js.name { - JsImportName::Global { name } => { - self.generate_identifier(name); + JsImportName::Global { .. } => { + self.import_name(js)?; } _ => {} } From 16a4a8bedc394f754337a7c5cb6faae7ab130333 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 15 Apr 2020 06:05:47 -0700 Subject: [PATCH 09/10] Add back in error handling, clean up instruction iteration --- crates/cli-support/src/js/mod.rs | 58 +++++++++++++------------------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 959202a96c1..81b41a9663b 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -1994,7 +1994,13 @@ impl<'a> Context<'a> { format!("l{}", name) } - JsImportName::Global { name } => self.generate_identifier(name), + JsImportName::Global { name } => { + let unique_name = self.generate_identifier(name); + if unique_name != *name { + bail!("cannot import `{}` from two locations", name); + } + unique_name + } }; self.imported_names .insert(import.name.clone(), name.clone()); @@ -2076,42 +2082,24 @@ impl<'a> Context<'a> { Ok(()) } + /// Registers import names for all `Global` imports first before we actually + /// process any adapters. + /// + /// `Global` names must be imported as their exact name, so if the same name + /// from a global is also imported from a module we have to be sure to + /// import the global first to ensure we don't shadow the actual global + /// value. Otherwise we have no way of accessing the global value! + /// + /// This function will iterate through the import map up-front and generate + /// a cache entry for each import name which is a `Global`. fn prestore_global_import_identifiers(&mut self) -> Result<(), Error> { - for (_id, adapter) in crate::sorted_iter(&self.wit.adapters) { - let instrs = match &adapter.kind { - AdapterKind::Import { .. } => continue, - AdapterKind::Local { instructions } => instructions, + for import in self.aux.import_map.values() { + let js = match import { + AuxImport::Value(AuxValue::Bare(js)) => js, + _ => continue, }; - let mut call = None; - for instr in instrs { - match instr.instr { - Instruction::CallAdapter(id) => { - if call.is_some() { - continue; - } else { - call = Some(id); - } - } - Instruction::CallExport(_) - | Instruction::CallTableElement(_) - | Instruction::Standard(wit_walrus::Instruction::CallCore(_)) - | Instruction::Standard(wit_walrus::Instruction::CallAdapter(_)) => continue, - _ => {} - } - } - if let Some(id) = call { - let js = match &self.aux.import_map[&id] { - AuxImport::Value(AuxValue::Bare(js)) => Some(js), - _ => None, - }; - if let Some(js) = js { - match &js.name { - JsImportName::Global { .. } => { - self.import_name(js)?; - } - _ => {} - } - } + if let JsImportName::Global { .. } = js.name { + self.import_name(js)?; } } Ok(()) From 7b5d994b2a46503fe4f1e080e0603884bc5a73e8 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 15 Apr 2020 06:06:44 -0700 Subject: [PATCH 10/10] Remove unnecessary patch statements --- examples/webxr/Cargo.toml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/examples/webxr/Cargo.toml b/examples/webxr/Cargo.toml index 00de3d1738b..1d261b7bda3 100644 --- a/examples/webxr/Cargo.toml +++ b/examples/webxr/Cargo.toml @@ -63,10 +63,3 @@ features = [ 'XrWebGlLayerInit', 'console' ] - - -[patch.crates-io] -wasm-bindgen = { path = '../../wasm-bindgen' } -wasm-bindgen-futures = { path = '../../wasm-bindgen/crates/futures' } -js-sys = { path = '../../wasm-bindgen/crates/js-sys' } -web-sys = { path = '../../wasm-bindgen/crates/web-sys' }