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.

Sign up for GitHub

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

Fix an issue where closure rewriting required class internals #1175

Merged
merged 3 commits into from
Jan 14, 2019
Merged

Fix an issue where closure rewriting required class internals #1175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0ddabc
Sanity check `exposed_globals` map
alexcrichton Jan 14, 2019
b214893
Ensure internal `exported_classes` map is consistent
alexcrichton Jan 14, 2019
20e871f
Fix an issue where closure rewriting required class internals
alexcrichton Jan 14, 2019
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
80 changes: 46 additions & 34 deletions crates/cli-support/src/js/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct Context<'a> {
pub imports_post: String,
pub footer: String,
pub typescript: String,
pub exposed_globals: HashSet<&'static str>,
pub exposed_globals: Option<HashSet<&'static str>>,
pub required_internal_exports: HashSet<&'static str>,
pub imported_functions: HashSet<&'a str>,
pub imported_statics: HashSet<&'a str>,
Expand Down Expand Up @@ -55,7 +55,7 @@ pub struct Context<'a> {
/// wasm-bindgen emits.
pub direct_imports: HashMap<&'a str, (&'a str, &'a str)>,

pub exported_classes: HashMap<String, ExportedClass>,
pub exported_classes: Option<HashMap<String, ExportedClass>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, this second commit is exactly the pattern that I was suggesting for the first one :-p

pub function_table_needed: bool,
pub interpreter: &'a mut Interpreter,
pub memory_init: Option<ResizableLimits>,
Expand Down Expand Up @@ -119,6 +119,10 @@ const INITIAL_HEAP_VALUES: &[&str] = &["undefined", "null", "true", "false"];
const INITIAL_HEAP_OFFSET: usize = 32;

impl<'a> Context<'a> {
fn should_write_global(&mut self, name: &'static str) -> bool {
self.exposed_globals.as_mut().unwrap().insert(name)
}

fn export(&mut self, name: &str, contents: &str, comments: Option<String>) {
let contents = contents.trim();
if let Some(ref c) = comments {
Expand Down Expand Up @@ -167,8 +171,6 @@ impl<'a> Context<'a> {
}

pub fn finalize(&mut self, module_name: &str) -> Result<(String, String), Error> {
self.write_classes()?;

self.bind("__wbindgen_object_clone_ref", &|me| {
me.expose_get_object();
me.expose_add_heap_object();
Expand Down Expand Up @@ -425,6 +427,7 @@ impl<'a> Context<'a> {
})?;

closures::rewrite(self).with_context(|_| "failed to generate internal closure shims")?;
self.write_classes()?;
self.unexport_unused_internal_exports();

// Handle the `start` function, if one was specified. If we're in a
Expand Down Expand Up @@ -478,6 +481,10 @@ impl<'a> Context<'a> {
self.rewrite_imports(module_name);
self.update_producers_section();

// Cause any future calls to `should_write_global` to panic, making sure
// we don't ask for items which we can no longer emit.
drop(self.exposed_globals.take().unwrap());

let mut js = if self.config.threads.is_some() {
// TODO: It's not clear right now how to best use threads with
// bundlers like webpack. We need a way to get the existing
Expand Down Expand Up @@ -651,8 +658,7 @@ impl<'a> Context<'a> {
}

fn write_classes(&mut self) -> Result<(), Error> {
let classes = mem::replace(&mut self.exported_classes, Default::default());
for (class, exports) in classes {
for (class, exports) in self.exported_classes.take().unwrap() {
self.write_class(&class, &exports)?;
}
Ok(())
Expand Down Expand Up @@ -874,7 +880,7 @@ impl<'a> Context<'a> {
}

fn expose_drop_ref(&mut self) {
if !self.exposed_globals.insert("drop_ref") {
if !self.should_write_global("drop_ref") {
return;
}
self.expose_global_heap();
Expand All @@ -901,7 +907,7 @@ impl<'a> Context<'a> {
}

fn expose_global_heap(&mut self) {
if !self.exposed_globals.insert("heap") {
if !self.should_write_global("heap") {
return;
}
self.global(&format!("const heap = new Array({});", INITIAL_HEAP_OFFSET));
Expand All @@ -910,15 +916,15 @@ impl<'a> Context<'a> {
}

fn expose_global_heap_next(&mut self) {
if !self.exposed_globals.insert("heap_next") {
if !self.should_write_global("heap_next") {
return;
}
self.expose_global_heap();
self.global("let heap_next = heap.length;");
}

fn expose_get_object(&mut self) {
if !self.exposed_globals.insert("get_object") {
if !self.should_write_global("get_object") {
return;
}
self.expose_global_heap();
Expand All @@ -929,7 +935,7 @@ impl<'a> Context<'a> {
}

fn expose_assert_num(&mut self) {
if !self.exposed_globals.insert("assert_num") {
if !self.should_write_global("assert_num") {
return;
}
self.global(&format!(
Expand All @@ -942,7 +948,7 @@ impl<'a> Context<'a> {
}

fn expose_assert_bool(&mut self) {
if !self.exposed_globals.insert("assert_bool") {
if !self.should_write_global("assert_bool") {
return;
}
self.global(&format!(
Expand All @@ -957,14 +963,14 @@ impl<'a> Context<'a> {
}

fn expose_wasm_vector_len(&mut self) {
if !self.exposed_globals.insert("wasm_vector_len") {
if !self.should_write_global("wasm_vector_len") {
return;
}
self.global("let WASM_VECTOR_LEN = 0;");
}

fn expose_pass_string_to_wasm(&mut self) -> Result<(), Error> {
if !self.exposed_globals.insert("pass_string_to_wasm") {
if !self.should_write_global("pass_string_to_wasm") {
return Ok(());
}
self.require_internal_export("__wbindgen_malloc")?;
Expand Down Expand Up @@ -1025,7 +1031,7 @@ impl<'a> Context<'a> {
}

fn expose_pass_array_jsvalue_to_wasm(&mut self) -> Result<(), Error> {
if !self.exposed_globals.insert("pass_array_jsvalue") {
if !self.should_write_global("pass_array_jsvalue") {
return Ok(());
}
self.require_internal_export("__wbindgen_malloc")?;
Expand Down Expand Up @@ -1054,7 +1060,7 @@ impl<'a> Context<'a> {
delegate: &str,
size: usize,
) -> Result<(), Error> {
if !self.exposed_globals.insert(name) {
if !self.should_write_global(name) {
return Ok(());
}
self.require_internal_export("__wbindgen_malloc")?;
Expand All @@ -1076,14 +1082,14 @@ impl<'a> Context<'a> {
}

fn expose_text_encoder(&mut self) {
if !self.exposed_globals.insert("text_encoder") {
if !self.should_write_global("text_encoder") {
return;
}
self.expose_text_processor("TextEncoder");
}

fn expose_text_decoder(&mut self) {
if !self.exposed_globals.insert("text_decoder") {
if !self.should_write_global("text_decoder") {
return;
}
self.expose_text_processor("TextDecoder");
Expand Down Expand Up @@ -1112,7 +1118,7 @@ impl<'a> Context<'a> {
}

fn expose_get_string_from_wasm(&mut self) {
if !self.exposed_globals.insert("get_string_from_wasm") {
if !self.should_write_global("get_string_from_wasm") {
return;
}
self.expose_text_decoder();
Expand Down Expand Up @@ -1148,7 +1154,7 @@ impl<'a> Context<'a> {
}

fn expose_get_array_js_value_from_wasm(&mut self) {
if !self.exposed_globals.insert("get_array_js_value_from_wasm") {
if !self.should_write_global("get_array_js_value_from_wasm") {
return;
}
self.expose_uint32_memory();
Expand Down Expand Up @@ -1224,7 +1230,7 @@ impl<'a> Context<'a> {
}

fn arrayget(&mut self, name: &'static str, mem: &'static str, size: usize) {
if !self.exposed_globals.insert(name) {
if !self.should_write_global(name) {
return;
}
self.global(&format!(
Expand Down Expand Up @@ -1341,7 +1347,7 @@ impl<'a> Context<'a> {
}

fn memview(&mut self, name: &'static str, js: &str) {
if !self.exposed_globals.insert(name) {
if !self.should_write_global(name) {
return;
}
let mem = self.memory();
Expand All @@ -1362,7 +1368,7 @@ impl<'a> Context<'a> {
}

fn expose_assert_class(&mut self) {
if !self.exposed_globals.insert("assert_class") {
if !self.should_write_global("assert_class") {
return;
}
self.global(
Expand All @@ -1378,14 +1384,14 @@ impl<'a> Context<'a> {
}

fn expose_global_stack_pointer(&mut self) {
if !self.exposed_globals.insert("stack_pointer") {
if !self.should_write_global("stack_pointer") {
return;
}
self.global(&format!("let stack_pointer = {};", INITIAL_HEAP_OFFSET));
}

fn expose_borrowed_objects(&mut self) {
if !self.exposed_globals.insert("borrowed_objects") {
if !self.should_write_global("borrowed_objects") {
return;
}
self.expose_global_heap();
Expand All @@ -1407,7 +1413,7 @@ impl<'a> Context<'a> {
}

fn expose_take_object(&mut self) {
if !self.exposed_globals.insert("take_object") {
if !self.should_write_global("take_object") {
return;
}
self.expose_get_object();
Expand All @@ -1424,7 +1430,7 @@ impl<'a> Context<'a> {
}

fn expose_add_heap_object(&mut self) {
if !self.exposed_globals.insert("add_heap_object") {
if !self.should_write_global("add_heap_object") {
return;
}
self.expose_global_heap();
Expand Down Expand Up @@ -1566,7 +1572,7 @@ impl<'a> Context<'a> {
}

fn expose_global_argument_ptr(&mut self) -> Result<(), Error> {
if !self.exposed_globals.insert("global_argument_ptr") {
if !self.should_write_global("global_argument_ptr") {
return Ok(());
}
self.require_internal_export("__wbindgen_global_argument_ptr")?;
Expand All @@ -1585,7 +1591,7 @@ impl<'a> Context<'a> {
}

fn expose_get_inherited_descriptor(&mut self) {
if !self.exposed_globals.insert("get_inherited_descriptor") {
if !self.should_write_global("get_inherited_descriptor") {
return;
}
// It looks like while rare some browsers will move descriptors up the
Expand Down Expand Up @@ -1613,7 +1619,7 @@ impl<'a> Context<'a> {

fn expose_u32_cvt_shim(&mut self) -> &'static str {
let name = "u32CvtShim";
if !self.exposed_globals.insert(name) {
if !self.should_write_global(name) {
return name;
}
self.global(&format!("const {} = new Uint32Array(2);", name));
Expand All @@ -1622,7 +1628,7 @@ impl<'a> Context<'a> {

fn expose_int64_cvt_shim(&mut self) -> &'static str {
let name = "int64CvtShim";
if !self.exposed_globals.insert(name) {
if !self.should_write_global(name) {
return name;
}
let n = self.expose_u32_cvt_shim();
Expand All @@ -1635,7 +1641,7 @@ impl<'a> Context<'a> {

fn expose_uint64_cvt_shim(&mut self) -> &'static str {
let name = "uint64CvtShim";
if !self.exposed_globals.insert(name) {
if !self.should_write_global(name) {
return name;
}
let n = self.expose_u32_cvt_shim();
Expand All @@ -1647,7 +1653,7 @@ impl<'a> Context<'a> {
}

fn expose_is_like_none(&mut self) {
if !self.exposed_globals.insert("is_like_none") {
if !self.should_write_global("is_like_none") {
return;
}
self.global(
Expand All @@ -1660,7 +1666,7 @@ impl<'a> Context<'a> {
}

fn expose_cleanup_groups(&mut self) {
if !self.exposed_globals.insert("cleanup_groups") {
if !self.should_write_global("cleanup_groups") {
return;
}
self.global(
Expand Down Expand Up @@ -1741,6 +1747,8 @@ impl<'a> Context<'a> {

fn require_class_wrap(&mut self, class: &str) {
self.exported_classes
.as_mut()
.expect("classes already written")
.entry(class.to_string())
.or_insert_with(ExportedClass::default)
.wrap_needed = true;
Expand Down Expand Up @@ -2301,6 +2309,8 @@ impl<'a, 'b> SubContext<'a, 'b> {
let class = self
.cx
.exported_classes
.as_mut()
.expect("classes already written")
.entry(class_name.to_string())
.or_insert(ExportedClass::default());
class
Expand Down Expand Up @@ -2530,6 +2540,8 @@ impl<'a, 'b> SubContext<'a, 'b> {
let class = self
.cx
.exported_classes
.as_mut()
.expect("classes already written")
.entry(struct_.name.to_string())
.or_insert_with(Default::default);
class.comments = format_doc_comments(&struct_.comments, None);
Expand Down
4 changes: 2 additions & 2 deletions crates/cli-support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,11 @@ impl Bindgen {
imports_post: String::new(),
footer: String::new(),
typescript: format!("/* tslint:disable */\n"),
exposed_globals: Default::default(),
exposed_globals: Some(Default::default()),
required_internal_exports: Default::default(),
imported_names: Default::default(),
imported_identifiers: Default::default(),
exported_classes: Default::default(),
exported_classes: Some(Default::default()),
config: &self,
module: &mut module,
function_table_needed: false,
Expand Down
5 changes: 5 additions & 0 deletions tests/wasm/closures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const assert = require('assert');
const wasm = require('wasm-bindgen-test');

exports.works_call = a => {
a();
Expand Down Expand Up @@ -97,3 +98,7 @@ exports.drop_during_call_save = f => {
DROP_DURING_CALL = f;
};
exports.drop_during_call_call = () => DROP_DURING_CALL();

exports.js_test_closure_returner = () => {
wasm.closure_returner().someKey();
};
Loading