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.

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

Preserve argument names #1344

Merged
merged 3 commits into from
Mar 14, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
23 changes: 22 additions & 1 deletion crates/backend/src/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,22 @@ fn shared_export<'a>(export: &'a ast::Export, intern: &'a Interner) -> Export<'a
}

fn shared_function<'a>(func: &'a ast::Function, _intern: &'a Interner) -> Function<'a> {
Function { name: &func.name }
let arg_names = func
.arguments
.iter()
.enumerate()
.map(|(idx, arg)| {
if let syn::Pat::Ident(ref x) = arg.pat {
x.ident.to_string()
} else {
format!("arg{}", idx)
}
})
.collect::<Vec<_>>();
Function {
name: &func.name,
arg_names,
}
}

fn shared_enum<'a>(e: &'a ast::Enum, intern: &'a Interner) -> Enum<'a> {
Expand Down Expand Up @@ -364,6 +379,12 @@ impl<'a> Encode for &'a str {
}
}

impl<'a> Encode for String {
fn encode(&self, dst: &mut Encoder) {
self.as_bytes().encode(dst);
}
}

impl<T: Encode> Encode for Vec<T> {
fn encode(&self, dst: &mut Encoder) {
self.len().encode(dst);
Expand Down
9 changes: 9 additions & 0 deletions crates/cli-support/src/decode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ impl<'src> Decode<'src> for &'src str {
}
}

impl<'src> Decode<'src> for String {
fn decode(data: &mut &'src [u8]) -> String {
let n = u32::decode(data);
let (a, b) = data.split_at(n as usize);
*data = b;
String::from_utf8(a.to_vec()).unwrap()
}
}

impl<'src, T: Decode<'src>> Decode<'src> for Vec<T> {
fn decode(data: &mut &'src [u8]) -> Self {
let n = u32::decode(data);
Expand Down
2 changes: 1 addition & 1 deletion crates/cli-support/src/js/closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl ClosureDescriptors {
builder.rust_argument("this.a").rust_argument("b");
}
builder.finally("if (this.cnt-- == 1) d(this.a, b);");
builder.process(&closure.function)?.finish(
builder.process(&closure.function, None)?.finish(
"function",
"f",
ExportedShim::TableElement(&mut shim),
Expand Down
36 changes: 28 additions & 8 deletions crates/cli-support/src/js/js2rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,22 @@ impl<'a, 'b> Js2Rust<'a, 'b> {

/// Generates all bindings necessary for the signature in `Function`,
/// creating necessary argument conversions and return value processing.
pub fn process(&mut self, function: &Function) -> Result<&mut Self, Error> {
for arg in function.arguments.iter() {
self.argument(arg)?;
pub fn process<'c, I>(
&mut self,
function: &Function,
opt_arg_names: I,
) -> Result<&mut Self, Error>
where
I: Into<Option<&'c Vec<String>>>,
{
if let Some(arg_names) = opt_arg_names.into() {
for (arg, arg_name) in function.arguments.iter().zip(arg_names) {
self.argument(arg, arg_name.clone())?;
}
} else {
for arg in function.arguments.iter() {
self.argument(arg, None)?;
}
}
self.ret(&function.ret)?;
Ok(self)
Expand Down Expand Up @@ -138,15 +151,22 @@ impl<'a, 'b> Js2Rust<'a, 'b> {
self
}

fn abi_arg(&mut self) -> String {
let s = format!("arg{}", self.arg_idx);
fn abi_arg(&mut self, opt_arg_name: Option<String>) -> String {
alexcrichton marked this conversation as resolved.
Show resolved Hide resolved
let ret = if let Some(x) = opt_arg_name {
x
} else {
format!("arg{}", self.arg_idx)
};
self.arg_idx += 1;
s
ret
}

pub fn argument(&mut self, arg: &Descriptor) -> Result<&mut Self, Error> {
pub fn argument<I>(&mut self, arg: &Descriptor, opt_arg_name: I) -> Result<&mut Self, Error>
where
I: Into<Option<String>>,
{
let i = self.arg_idx;
let name = self.abi_arg();
let name = self.abi_arg(opt_arg_name.into());

let (arg, optional) = match arg {
Descriptor::Option(t) => (&**t, true),
Expand Down
6 changes: 3 additions & 3 deletions crates/cli-support/src/js/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2498,7 +2498,7 @@ impl<'a, 'b> SubContext<'a, 'b> {
}

let (js, ts, js_doc) = Js2Rust::new(&export.function.name, self.cx)
.process(descriptor.unwrap_function())?
.process(descriptor.unwrap_function(), &export.function.arg_names)?
.finish(
"function",
&format!("wasm.{}", export.function.name),
Expand Down Expand Up @@ -2554,7 +2554,7 @@ impl<'a, 'b> SubContext<'a, 'b> {
} else {
None
})
.process(descriptor.unwrap_function())?
.process(descriptor.unwrap_function(), &export.function.arg_names)?
.finish(
"",
&format!("wasm.{}", wasm_name),
Expand Down Expand Up @@ -2772,7 +2772,7 @@ impl<'a, 'b> SubContext<'a, 'b> {
let setter = ExportedShim::Named(&wasm_setter);
let mut cx = Js2Rust::new(&field.name, self.cx);
cx.method(true, false)
.argument(&descriptor)?
.argument(&descriptor, None)?
.ret(&Descriptor::Unit)?;
ts_dst.push_str(&format!(
"\n {}{}: {};",
Expand Down
2 changes: 1 addition & 1 deletion crates/cli-support/src/js/rust2js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
} else {
builder.rust_argument("this.a");
}
builder.rust_argument("this.b").process(f)?.finish(
builder.rust_argument("this.b").process(f, None)?.finish(
"function",
"this.f",
ExportedShim::TableElement(&mut shim),
Expand Down
1 change: 1 addition & 0 deletions crates/shared/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ macro_rules! shared_api {
}

struct Function<'a> {
arg_names: Vec<String>,
Copy link
Contributor

Choose a reason for hiding this comment

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

If possible it'd be great to use &'a str here to allow the encoder/decoder to be pretty lean and avoid the extra String allocations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wish! syn's Ident doesn't expose &str and only implements ToString.

name: &'a str,
}

Expand Down
16 changes: 16 additions & 0 deletions tests/wasm/arg_names.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const wasm = require('wasm-bindgen-test.js');
const assert = require('assert');

const ARGUMENT_NAMES = /([^\s,]+)/g;
const STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;

// https://stackoverflow.com/q/1007981/210304
function getArgNames(func) {
let fnStr = func.toString().replace(STRIP_COMMENTS, '');
let result = fnStr.slice(fnStr.indexOf('(')+1, fnStr.indexOf(')')).match(ARGUMENT_NAMES);
return result === null ? [] : result;
}

exports.js_arg_names = () => {
assert.deepEqual(getArgNames(wasm.fn_with_many_args), ['_a', '_b', '_c', '_d']);
};
15 changes: 15 additions & 0 deletions tests/wasm/arg_names.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
use wasm_bindgen::prelude::*;
use wasm_bindgen_test::*;

#[wasm_bindgen(module = "tests/wasm/arg_names.js")]
extern "C" {
fn js_arg_names();
}

#[wasm_bindgen]
pub fn fn_with_many_args(_a: i32, _b: i32, _c: i32, _d: i32) {}

#[wasm_bindgen_test]
fn rust_arg_names() {
js_arg_names();
}
1 change: 1 addition & 0 deletions tests/wasm/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ extern crate serde_derive;
use wasm_bindgen::prelude::*;

pub mod api;
pub mod arg_names;
pub mod char;
pub mod classes;
pub mod closures;
Expand Down