Merge pull request #1175 from alexcrichton/internal-consistency

Fix an issue where closure rewriting required class internals
This commit is contained in:
Alex Crichton 2019-01-14 15:53:40 -08:00 committed by GitHub
commit 5c04427382
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 85 additions and 36 deletions

View File

@ -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>,
@ -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>,
@ -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 {
@ -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();
@ -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
@ -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
@ -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(())
@ -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();
@ -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));
@ -910,7 +916,7 @@ 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();
@ -918,7 +924,7 @@ impl<'a> Context<'a> {
}
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();
@ -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!(
@ -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!(
@ -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")?;
@ -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")?;
@ -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")?;
@ -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");
@ -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();
@ -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();
@ -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!(
@ -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();
@ -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(
@ -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();
@ -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();
@ -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();
@ -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")?;
@ -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
@ -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));
@ -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();
@ -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();
@ -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(
@ -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(
@ -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;
@ -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
@ -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);

View File

@ -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,

View File

@ -1,4 +1,5 @@
const assert = require('assert');
const wasm = require('wasm-bindgen-test');
exports.works_call = a => {
a();
@ -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();
};

View File

@ -63,6 +63,8 @@ extern "C" {
fn drop_during_call_save(a: &Closure<Fn()>);
fn drop_during_call_call();
fn js_test_closure_returner();
}
#[wasm_bindgen_test]
@ -279,3 +281,33 @@ fn drop_during_call_ok() {
assert!(HIT);
}
}
#[wasm_bindgen_test]
fn test_closure_returner() {
type ClosureType = FnMut() -> BadStruct;
use js_sys::{Object, Reflect};
use wasm_bindgen::JsCast;
js_test_closure_returner();
#[wasm_bindgen]
pub struct ClosureHandle(Closure<ClosureType>);
#[wasm_bindgen]
pub struct BadStruct {}
#[wasm_bindgen]
pub fn closure_returner() -> Result<Object, JsValue> {
let o = Object::new();
let some_fn = Closure::wrap(Box::new(move || BadStruct {}) as Box<ClosureType>);
Reflect::set(&o, &JsValue::from("someKey"), &some_fn.as_ref().unchecked_ref())
.unwrap();
Reflect::set(&o, &JsValue::from("handle"), &JsValue::from(ClosureHandle(some_fn)))
.unwrap();
Ok(o)
}
}