Second large refactor for WebIDL bindings #1594

alexcrichton · 2019-06-13T23:19:30Z

This commit is the second, and hopefully last massive, refactor for
using WebIDL bindings internally in wasm-bindgen. This commit actually
fully executes on the task at hand, moving wasm-bindgen to internally
using WebIDL bindings throughout its code generation, anyref passes,
etc. This actually fixes a number of issues that have existed in the
anyref pass for some time now!

The main changes here are to basically remove the usage of Descriptor
from generating JS bindings. Instead two new types are introduced:
NonstandardIncoming and NonstandardOutgoing which are bindings lists
used for incoming/outgoing bindings. These mirror the standard
terminology and literally have variants which are the standard values.
All Descriptor types are now mapped into lists of incoming/outgoing
bindings and used for process in wasm-bindgen. All JS generation has
been refactored and updated to now process these lists of bindings
instead of the previous Descriptor.

In other words this commit takes js2rust.rs and rust2js.rs and first
splits them in two. Interpretation of Descriptor and what to do for
conversions is in the binding selection modules. The actual generation
of JS from the binding selection is now performed by incoming.rs and
outgoing.rs. To boot this also deduplicates all the code between the
argument handling of js2rust.rs and return value handling of
rust2js.rs. This means that to implement a new binding you only need
to implement it one place and it's implemented for free in the other!

This commit is not the end of the story though. I would like to add a
mdoe to wasm-bindgen that literally emits a WebIDL bindings section.
That's left for a third (and hopefully final) refactoring which is also
intended to optimize generated JS for bindings.

This commit currently loses the optimization where an imported is hooked
up by value directly whenever a shim isn't needed. It's planned that
the next refactoring to emit a webidl binding section that can be added
back in. It shouldn't be too too hard hopefully since all the
scaffolding is in place now.

cc #1524

alexcrichton · 2019-06-13T23:22:17Z

I'll apologize in advance for the size of this as well. I didn't really find a great place to split up the changes here, it was sort of an all-or-nothing change to have anything working to a respectable degree of fidelity. @fitzgen please take your time of course in looking this over, there's no rush!

Hywan

Thanks for commenting such a massive PR, that's very helpful!

Hywan · 2019-06-17T08:23:39Z

crates/cli-support/src/anyref.rs

+
+    // Afterwards look through the argument list (specified with various
+    // bindings) to find any borrowed anyref values and update our
+    // transformation metadata accordingly. if we find one then the binding no


Hywan · 2019-06-17T08:24:24Z

crates/cli-support/src/anyref.rs

+    // Afterwards look through the argument list (specified with various
+    // bindings) to find any borrowed anyref values and update our
+    // transformation metadata accordingly. if we find one then the binding no
+    // longer needs to remember its borrowed but rather it's just a simple cast


- its borrowed + it's borrowed

Hywan · 2019-06-17T08:31:11Z

crates/cli-support/src/js/binding.rs

+        }
+
+        // First up we handle all the arguments. Depending on whether incoming
+        // or outgoing ar the arguments this is pretty different.


This commit is the second, and hopefully last massive, refactor for using WebIDL bindings internally in `wasm-bindgen`. This commit actually fully executes on the task at hand, moving `wasm-bindgen` to internally using WebIDL bindings throughout its code generation, anyref passes, etc. This actually fixes a number of issues that have existed in the anyref pass for some time now! The main changes here are to basically remove the usage of `Descriptor` from generating JS bindings. Instead two new types are introduced: `NonstandardIncoming` and `NonstandardOutgoing` which are bindings lists used for incoming/outgoing bindings. These mirror the standard terminology and literally have variants which are the standard values. All `Descriptor` types are now mapped into lists of incoming/outgoing bindings and used for process in wasm-bindgen. All JS generation has been refactored and updated to now process these lists of bindings instead of the previous `Descriptor`. In other words this commit takes `js2rust.rs` and `rust2js.rs` and first splits them in two. Interpretation of `Descriptor` and what to do for conversions is in the binding selection modules. The actual generation of JS from the binding selection is now performed by `incoming.rs` and `outgoing.rs`. To boot this also deduplicates all the code between the argument handling of `js2rust.rs` and return value handling of `rust2js.rs`. This means that to implement a new binding you only need to implement it one place and it's implemented for free in the other! This commit is not the end of the story though. I would like to add a mdoe to `wasm-bindgen` that literally emits a WebIDL bindings section. That's left for a third (and hopefully final) refactoring which is also intended to optimize generated JS for bindings. This commit currently loses the optimization where an imported is hooked up by value directly whenever a shim isn't needed. It's planned that the next refactoring to emit a webidl binding section that can be added back in. It shouldn't be too too hard hopefully since all the scaffolding is in place now. cc rustwasm#1524

fitzgen

Alex and I were in the same location last week, and we talked through this PR in person. I'm excited! Next step will be actually using the wasm_webidl_bindings::WebidlBindings custom section type internally :)

alexcrichton · 2019-06-25T06:21:27Z

🎊

After rustwasm#1594 constructors of Rust exported structs started using class wrapping when generating JS shims. Wrapping erases prototype information from the object instance in JS and as a result it is not possible to override methods (via inheritance) of the generated class. Additionally, some checks to ensure constructors always return an instance of `Self` were lost. This PR fixes the above two issues by embedding the kind of export into the `CallExport` instruction and monitoring for constructor exports calls during the export adapter creation. Once a constructor export call is detected and validated a new instruction `SelfFromI32` is pushed into the stack that avoids class wrapping. Fixes rustwasm#3213 Signed-off-by: Oliver T <geronimooliver00@gmail.com>

After rustwasm#1594 constructors of Rust exported structs started using class wrapping when generating JS shims. Wrapping erases prototype information from the object instance in JS and as a result it is not possible to override methods (via inheritance) of the generated class. Additionally, some checks to ensure constructors always return an instance of `Self` were lost. This PR is rebased from rustwasm#3561, it passes the constructor information from the `Builder` into the instruction translation function which is then used to modify the generated bindings. The `return` statement is also removed instead the value is directly attached to the instance. Signed-off-by: Oliver T <geronimooliver00@gmail.com>

* Remove class wrap for constructors in Rust exports After #1594 constructors of Rust exported structs started using class wrapping when generating JS shims. Wrapping erases prototype information from the object instance in JS and as a result it is not possible to override methods (via inheritance) of the generated class. Additionally, some checks to ensure constructors always return an instance of `Self` were lost. This PR is rebased from #3561, it passes the constructor information from the `Builder` into the instruction translation function which is then used to modify the generated bindings. The `return` statement is also removed instead the value is directly attached to the instance. Signed-off-by: Oliver T <geronimooliver00@gmail.com> * Fix typo Co-authored-by: Liam Murphy <liampm32@gmail.com> * Disallow returning JS primitives from constructors Signed-off-by: Oliver T <geronimooliver00@gmail.com> * Added missing String in match Signed-off-by: Oliver T <geronimooliver00@gmail.com> * Handle nested descriptors Signed-off-by: Oliver T <geronimooliver00@gmail.com> --------- Signed-off-by: Oliver T <geronimooliver00@gmail.com> Co-authored-by: Liam Murphy <liampm32@gmail.com>

alexcrichton requested a review from fitzgen June 13, 2019 23:19

alexcrichton mentioned this pull request Jun 13, 2019

Moving wasm-bindgen from "Future Compatible with" to "Polyfill for" WebIDL Bindings #1524

Closed

4 tasks

alexcrichton force-pushed the webidl-for-realz branch from 6fb9d54 to d0f3f39 Compare June 14, 2019 18:47

This was referenced Jun 14, 2019

Statically rule out "weird" incoming binding expression WebAssembly/interface-types#37

Closed

Apply BindExport to table elements as well WebAssembly/interface-types#35

Closed

Hywan reviewed Jun 17, 2019

View reviewed changes

alexcrichton force-pushed the webidl-for-realz branch from 0b49a63 to 0196dfb Compare June 20, 2019 00:58

alexcrichton force-pushed the webidl-for-realz branch from 0196dfb to 3cc3084 Compare June 21, 2019 02:16

fitzgen approved these changes Jun 24, 2019

View reviewed changes

alexcrichton merged commit e0ef329 into rustwasm:master Jun 25, 2019

alexcrichton deleted the webidl-for-realz branch June 25, 2019 06:21

Liamolucko mentioned this pull request Aug 5, 2023

Cannot inherit from structs created via #[wasm_bindgen] attribute as expected #3213

Closed

snOm3ad mentioned this pull request Aug 17, 2023

Remove class wrap for constructors in Rust exports #3561

Closed

snOm3ad mentioned this pull request Aug 18, 2023

Remove class wrap for constructors in Rust exports #3562

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Second large refactor for WebIDL bindings #1594

Second large refactor for WebIDL bindings #1594

alexcrichton commented Jun 13, 2019

alexcrichton commented Jun 13, 2019

Hywan left a comment

Hywan Jun 17, 2019

Hywan Jun 17, 2019

Hywan Jun 17, 2019

fitzgen left a comment

alexcrichton commented Jun 25, 2019

Second large refactor for WebIDL bindings #1594

Second large refactor for WebIDL bindings #1594

Conversation

alexcrichton commented Jun 13, 2019

alexcrichton commented Jun 13, 2019

Hywan left a comment

Choose a reason for hiding this comment

Hywan Jun 17, 2019

Choose a reason for hiding this comment

Hywan Jun 17, 2019

Choose a reason for hiding this comment

Hywan Jun 17, 2019

Choose a reason for hiding this comment

fitzgen left a comment

Choose a reason for hiding this comment

alexcrichton commented Jun 25, 2019