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

Jump to bottom

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 1 commit
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
66 changes: 38 additions & 28 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 @@ -21,6 +21,7 @@ mod closures;

pub struct Context<'a> {
pub globals: String,
pub globals_written: bool,
pub imports: String,
pub imports_post: String,
pub footer: String,
Expand Down Expand Up @@ -119,6 +120,11 @@ 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 {
assert!(!self.globals_written);
self.exposed_globals.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 @@ -478,6 +484,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.
self.globals_written = true;
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, exposed_globals could be an Option<...> and then we can have should_write_global do self.exposed_globals.as_mut().unwrap().insert(name) and then here do self.exposed_globals.take().unwrap().

This is equivalent to having a bool flag (which becomes the option discriminant in this scheme) but it is impossible to forget to check the flag in should_write_global or anything else that wants to touch exposed_globals thanks to the type system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍


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 @@ -874,7 +884,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 +911,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 +920,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 +939,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 +952,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 +967,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 +1035,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 +1064,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 +1086,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 +1122,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 +1158,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 +1234,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 +1351,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 +1372,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 +1388,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 +1417,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 +1434,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 +1576,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 +1595,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 +1623,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 +1632,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 +1645,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 +1657,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 +1670,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
1 change: 1 addition & 0 deletions crates/cli-support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ impl Bindgen {
let (js, ts) = {
let mut cx = js::Context {
globals: String::new(),
globals_written: false,
imports: String::new(),
imports_post: String::new(),
footer: String::new(),
Expand Down