Skip to content

Commit

Permalink
Merge pull request rustwasm#1175 from alexcrichton/internal-consistency
Browse files Browse the repository at this point in the history 
Fix an issue where closure rewriting required class internals
  • Loading branch information
@alexcrichton
alexcrichton authored Jan 14, 2019
2 parents 1526d18 + 20e871f commit 5c04427
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 36 deletions.
80 changes: 46 additions & 34 deletions crates/cli-support/src/js/mod.rs
View file
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>>,
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
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
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

0 comments on commit 5c04427

Please sign in to comment.