Merge pull request #1805 from fitzgen/wasm-interface-types-and-multi-value

Wasm interface types and multi-value

Author: fitzgen

crates/cli-support/src/lib.rs

@@ -96,7 +96,7 @@ impl Bindgen {
             weak_refs: env::var("WASM_BINDGEN_WEAKREF").is_ok(),
             threads: threads_config(),
             anyref: anyref || wasm_interface_types,
-            multi_value,
+            multi_value: multi_value || wasm_interface_types,
             wasm_interface_types,
             encode_into: EncodeInto::Test,
         }

crates/cli-support/src/webidl/mod.rs

@@ -954,7 +954,10 @@ impl<'a> Context<'a> {
         match op.kind {
             decode::OperationKind::Regular => {
                 if op.is_static {
-                    Ok((AuxImport::ValueWithThis(class, function.name.to_string()), false))
+                    Ok((
+                        AuxImport::ValueWithThis(class, function.name.to_string()),
+                        false,
+                    ))
                 } else if structural {
                     Ok((
                         AuxImport::StructuralMethod(function.name.to_string()),
@@ -1361,48 +1364,26 @@ impl<'a> Context<'a> {
             .bindings
             .get(bind.binding)
             .ok_or_else(|| format_err!("bad binding id"))?;
-        let mut return_via_outptr = None;
         let (wasm_ty, webidl_ty, incoming, outgoing) = match binding {
-            ast::FunctionBinding::Export(e) => {
-                // This `match` is weird, see the comment at the top of
-                // `standard.rs` for what it's doing.
-                let outgoing = match e.result.bindings.get(0) {
-                    Some(ast::OutgoingBindingExpression::As(a)) if a.idx == u32::max_value() => {
-                        return_via_outptr = Some(vec![walrus::ValType::I32, walrus::ValType::I32]);
-                        &e.result.bindings[1..]
-                    }
-                    _ => &e.result.bindings[..],
-                };
-                (
-                    e.wasm_ty,
-                    e.webidl_ty,
-                    e.params.bindings.as_slice(),
-                    outgoing,
-                )
-            }
-            ast::FunctionBinding::Import(e) => {
-                // This `match` is weird, see the comment at the top of
-                // `standard.rs` for what it's doing.
-                let incoming = match e.result.bindings.get(0) {
-                    Some(ast::IncomingBindingExpression::Get(g)) if g.idx == u32::max_value() => {
-                        return_via_outptr = Some(vec![walrus::ValType::I32, walrus::ValType::I32]);
-                        &e.result.bindings[1..]
-                    }
-                    _ => &e.result.bindings[..],
-                };
-                (
-                    e.wasm_ty,
-                    e.webidl_ty,
-                    incoming,
-                    e.params.bindings.as_slice(),
-                )
-            }
+            ast::FunctionBinding::Export(e) => (
+                e.wasm_ty,
+                e.webidl_ty,
+                e.params.bindings.as_slice(),
+                &e.result.bindings[..],
+            ),
+            ast::FunctionBinding::Import(e) => (
+                e.wasm_ty,
+                e.webidl_ty,
+                &e.result.bindings[..],
+                e.params.bindings.as_slice(),
+            ),
         };
         let webidl_ty = standard::copy_ty(&mut self.bindings.types, webidl_ty, &std.types);
         let webidl_ty = match webidl_ty {
             ast::WebidlTypeRef::Id(id) => <ast::WebidlFunction as ast::WebidlTypeId>::wrap(id),
             _ => bail!("invalid webidl type listed"),
         };
+
         Ok(Binding {
             wasm_ty,
             webidl_ty,
@@ -1416,7 +1397,7 @@ impl<'a> Context<'a> {
                 .cloned()
                 .map(NonstandardOutgoing::Standard)
                 .collect(),
-            return_via_outptr,
+            return_via_outptr: None,
         })
     }
 

crates/cli-support/src/webidl/standard.rs

@@ -16,39 +16,6 @@
 //! generating any JS glue. Any JS glue currently generated is also invalid if
 //! the module contains the wasm bindings section and it's actually respected.
 
-// NB: Returning strings is weird
-//
-// This module has what is currently a pretty gross hack for dealing with
-// returning strings. One of the banner features of WebIDL bindings is not
-// requiring any language-specific glue to use wasm files and you get all sorts
-// of types like integers and strings by default. Note that strings are a huge
-// thing here.
-//
-// Dealing with *incoming* strings is easy enough in that the binding expression
-// has an allocator function to call and it's filled in and passed as two
-// pointers. Outgoing strings are a little harder, however, for two reasons:
-//
-// * One is that we need to return two values, which requires multi-value
-// * Another is that someone's got to free the string at some point
-//
-// Rust/wasm-bindgen don't support multi-value, and WebIDL bindings as literally
-// spec'd right this red hot second don't support freeing strings that we pass
-// out. These both have obvious fixes (supporting multi-value and extending the
-// bindings spec to support freeing) but we also want something working right
-// this red-hot second.
-//
-// To get things working we employ a terrible hack where the first bindign
-// expression of the result a function may indicate "use a thing that's way off
-// in the ether" and that's actually a sentinel for "oh ignore everything else
-// and the string is returned through an out-ptr as the first argument". This
-// manifests in all sorts of special hacks all over the place to handle this,
-// and it's a real bummer.
-//
-// This is in general just an explainer for the current state of things and
-// also a preemptive apology for writing the code to handle this in so many
-// places. I, like you, look forward to actually fixing this for real as the
-// spec continues to evolve and we implement more in wasm-bindgen.
-
 use crate::descriptor::VectorKind;
 use crate::webidl::{AuxExportKind, AuxImport, AuxValue, JsImport, JsImportName};
 use crate::webidl::{NonstandardIncoming, NonstandardOutgoing};
@@ -240,26 +207,15 @@ pub fn add_section(
                 name
             )
         })?;
-        let mut result = extract_outgoing(&binding.outgoing).with_context(|_| {
+        let result = extract_outgoing(&binding.outgoing).with_context(|_| {
             format!(
                 "failed to map return value for export `{}` to standard \
                  binding expressions",
                 name
             )
         })?;
 
-        // see comment at top of this module about returning strings for what
-        // this is doing and why it's weird
-        if binding.return_via_outptr.is_some() {
-            result.insert(
-                0,
-                ast::OutgoingBindingExpressionAs {
-                    idx: u32::max_value(),
-                    ty: ast::WebidlScalarType::Long.into(),
-                }
-                .into(),
-            );
-        }
+        assert!(binding.return_via_outptr.is_none());
         let binding = section.bindings.insert(ast::ExportBinding {
             wasm_ty: binding.wasm_ty,
             webidl_ty: copy_ty(
@@ -293,24 +249,14 @@ pub fn add_section(
                 module_name, name,
             )
         })?;
-        let mut result = extract_incoming(&binding.incoming).with_context(|_| {
+        let result = extract_incoming(&binding.incoming).with_context(|_| {
             format!(
                 "failed to map return value of import `{}::{}` to standard \
                  binding expressions",
                 module_name, name,
             )
         })?;
-        // see comment at top of this module about returning strings for what
-        // this is doing and why it's weird
-        if binding.return_via_outptr.is_some() {
-            result.insert(
-                0,
-                ast::IncomingBindingExpressionGet {
-                    idx: u32::max_value(),
-                }
-                .into(),
-            );
-        }
+        assert!(binding.return_via_outptr.is_none());
         let binding = section.bindings.insert(ast::ImportBinding {
             wasm_ty: binding.wasm_ty,
             webidl_ty: copy_ty(