Skip to content

Commit

Permalink
Merge pull request #1805 from fitzgen/wasm-interface-types-and-multi-…
Browse files Browse the repository at this point in the history 
…value

Wasm interface types and multi-value
  • Loading branch information
@fitzgen
fitzgen authored Oct 18, 2019
2 parents 46cbd19 + 25dd84c commit fe4dd0b
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 96 deletions.
2 changes: 1 addition & 1 deletion crates/cli-support/src/lib.rs
100644 → 100755
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
55 changes: 18 additions & 37 deletions crates/cli-support/src/webidl/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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,
Expand All @@ -1416,7 +1397,7 @@ impl<'a> Context<'a> {
.cloned()
.map(NonstandardOutgoing::Standard)
.collect(),
return_via_outptr,
return_via_outptr: None,
})
}

Expand Down
62 changes: 4 additions & 58 deletions crates/cli-support/src/webidl/standard.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit fe4dd0b

Please sign in to comment.