Proposal to support single inheritance and casting in wasm-bindgen

[fitzgen]

Rendered

[icefoxen]

My very un-expert two cents...

I like requiring explicit casting 'cause explicit is good. Using From and TryFrom instead feels more Rusty but loses semantic meaning.

Don't re-export the traits in prelude, one can always import them oneself as necessary. We can always change that later.

instanceof (or at least typeof) is often necessary in JS code and such, so generating helper functions for it might be nice.

[ohanar]

A few first thoughts:

• You don't always just want to upcast or downcast one level at a time for a deeply nested inheritance chain. Using an associated type on Upcast, you can only cast up or down one level at a time, so even in single inheritance I think we should probably have a trait parameter rather than an associated type for Upcast.
• I like the idea of using the standard conversion traits, and I don't really think that having all the extra impls is that much of an issue since everything is just being generated. On the other hand, if you have some a field in a borrowed struct you want to cast, it is a bit annoying and can be confusing to newcomers: (&foo.bar).into() vs foo.bar.upcast_ref(). Given this, I'm actually leaning towards specialize Upcast/Downcast traits, but it isn't a strong preference.
• Related to the previous point, we could both implement From/TryFrom traits as well Upcast/Downcast. This is already partially suggested in the RFC by including one impl for the From trait (for owned values). I would either go all the way, and implement all the relevant From/TryFrom and Upcast/Downcast, or make a conscious decision to use our own casting traits, and only implement the Upcast/Downcast traits.
• Regarding the instanceof helper functions, if it is exposed, we could make it a static method on the type that's being targeted, rather than having a bunch of loose functions. For instance you could call Foo::has_instance(&bar) to check if bar is an instance of Foo. Alternatively or in addition we could make it a method and just use the turbo fish: bar.instance_of::<Foo>().

[alexcrichton]

Thanks for the RFC @fitzgen!

I think may be important here to define in this RFC what role we think these traits and/or mechanisms will have on ecosystems API. On one end of the spectrum we could say that we want the solution here to be as ergonomic as possible, setting the precedent for how the entire ecosystem uses JS objects and casts. On the other end we could say that we only want to support the ability to connect types between their parent and child classes, but don't care about ergonomics at all.

I personally feel that we should probably lean towards the latter end of the spectrum, mainly empowering users here rather than defining ecosystem-wide conventions about how they're expected to be used. The web-sys crate, like libc, is intended to be the foundation of many crates. The libc crate, for example, doesn't attempt to set any trends about how to use APIs, exporting unsafe extern fn getpid() -> c_int which neither needs to be unsafe nor really needs to return a c_int as an extra layer of indirection (for example).

Along those lines I think I'd like to propose another alternative to this RFC. I feel that the Upcast and Downcast traits introduced here sort of sit in the middle of the spectrum mentioned above. One one hand I don't think they're suitable for ecosystem-wide ergonomics because I don't think you can define a function that takes any subclass of Node, and additionally for downcasting it doesn't currently (although it could certainly have them) have unchecked downcasts.

The alternative I'm thinking of looks like this:

pub unsafe trait AllJsValues {
    fn from_js_value(js: JsValue) -> Self where Self: Sized;
    fn into_js_value(self) -> JsValue where Self: Sized;
    
    fn cast<U>(self) -> U
        where U: AllJsValues + Sized, Self: Sized 
    {
        U::from_js_value(self.into_js_value())
    }
        
    fn cast_ref<U>(&self) -> &U
        where U: AllJsValues
    {
        unsafe { &*(self as *const Self as *const U) }
    }

    fn cast_mut<U>(&mut self) -> &mut U
        where U: AllJsValues
    {
        unsafe { &mut *(self as *mut Self as *mut U) }
    }
}

An implementation of this trait could be generated by #[wasm_bindgen] for all imported types. I see this version of the trait on the extreme end of "let's provide low level functionality and that's it". The idea here is that it allows you to cast between any types anywhere (as that's sort of in theory what you can do in JS). It still leaves room, however, for a perhaps more formal abstraction in the ecosystem.

We could also be even more "extreme" and purely provide accessors for the JsValue, &JsValue, and &mut JsValue internally, then providing constructors for all three as well for all types, meaning casts have to go into some sort of a JsValue and then get casted into a different type. (this could all be From traits I believe?)

In any case I'm personally a fan of sticking to the lower end of the spectrum of "not providing too much abstraction", but I'm curious what others think as well!

[fitzgen]

@ohanar

You don't always just want to upcast or downcast one level at a time for a deeply nested inheritance chain. Using an associated type on Upcast, you can only cast up or down one level at a time, so even in single inheritance I think we should probably have a trait parameter rather than an associated type for Upcast.

This is a really good insight!

My one fear is that we lose the type inference that allows chaining like

derived.upcast().base_method();

and instead are forced to write temporaries and help out type inference:

let base: MyBase = derived.upcast();
base.base_method();

I wonder if we can separate facts about inheritance relationships from the methods that do the casting so that we can maintain ergonomics. I think turbo-fishing is the best we could do syntax/ergonomics-wise with non-linear upcasting:

derived.upcast::<MyBase>().base_method();

This would require trait definitions like

pub unsafe trait Extends<T>: Sized + Into<T> {}

pub trait Upcast {
    fn upcast<T>(self) -> T
    where
        Self: Extends<T>;

    fn upcast_ref<T>(&self) -> &T
    where
        Self: Extends<T>;

    fn upcast_mut<T>(&mut self) -> &mut T
    where
        Self: Extends<T>;
}

impl<U> Upcast for U {
    #[inline]
    fn upcast<T>(self) -> T
    where
        Self: Extends<T>,
    {
        self.into()
    }

    #[inline]
    fn upcast_ref<T>(&self) -> &T
    where
        Self: Extends<T>,
    {
        unsafe { std::mem::transmute(self) }
    }

    #[inline]
    fn upcast_mut<T>(&mut self) -> &mut T
    where
        Self: Extends<T>,
    {
        unsafe { std::mem::transmute(self) }
    }
}

So basically it comes down to whether non-linear upcasting is worth having to turbofish in method chaining like derived.upcast::<MyBase>().base_method().

Thoughts?

(Playground with this version of the upcasting trait)

[fitzgen]

@alexcrichton

I think may be important here to define in this RFC what role we think these traits and/or mechanisms will have on ecosystems API. On one end of the spectrum we could say that we want the solution here to be as ergonomic as possible, setting the precedent for how the entire ecosystem uses JS objects and casts. On the other end we could say that we only want to support the ability to connect types between their parent and child classes, but don't care about ergonomics at all.

I personally feel that we should probably lean towards the latter end of the spectrum, mainly empowering users here rather than defining ecosystem-wide conventions about how they're expected to be used. The web-sys crate, like libc, is intended to be the foundation of many crates. The libc crate, for example, doesn't attempt to set any trends about how to use APIs, exporting unsafe extern fn getpid() -> c_int which neither needs to be unsafe nor really needs to return a c_int as an extra layer of indirection (for example).

Agreed that our number one goal is empowering users to leverage APIs that make use of inheritance, that ergonomics is a nice-to-have, and that setting conventions for all crates built on top of wasm-bindgen is a non-goal.

However, I would like to disentangle type safety from ergonomics.

libc is not type safe, and therefore raw, thin bindings to it are also not type safe.

On the other hand, Web APIs/WebIDL are type safe, and we shouldn't lose that property when writing raw, thin bindings to them.

I'm happy to include unsafe fn unchecked_cast<T: Into<JsValue>>(T) -> Self methods on imported types, but I don't think that alone is sufficient.

So looking at the problem once again from the perspective of

• type-safe casting as a hard requirement, and
• ergonomics of that casting as nice-to-have only when it comes "free"

then I don't think AllJsValues is a solution. But I think both the RFC's current Upcast and the Upcast from my last comment both fit these goals.

[alexcrichton]

Excellent points @fitzgen! I think it's worth digging into the type safety point here. I personally think we should be wary, though, of ascribing type safety to the synthetic types we're attaching to JS objects.

We've got WebIDL for the web, but how sure are we that it's 100% exactly what all browsers accept? Are we sure that there aren't at least a few corner cases of some "duck typed" APIs or otherwise instances of accepting more than one type for compatibility's sake?

Additionally, while WebIDL may be typesafe I'd imagine that the NPM ecosystem at large likely isn't in the sense that some types could be ascribed but at least in Typescript what I've seen is a lot of interfaces rather than classes (like a trait object instead of a concrete type) and there's not a great way to express that I think?

I think what I'm trying to say is that it feels to me like an already-lost uphill battle to ascribe types to the JS ecosystem with 100% fidelity in the sense that we can use unsafe for "you've basically created a segfault with this API". I would instead expect this to be 80-90% fidelity (or something like that) and there are cases where you'd need to cheat a bit with types. If we ascribe Rust's unsafe to this sort of "required cheating" it feels to me like a misuse of unsafe in the sense that unsafe is used to demarcate issues that can be caught with 100% fidelity.

[fitzgen]

I think what I'm trying to say is that it feels to me like an already-lost uphill battle to ascribe types to the JS ecosystem with 100% fidelity in the sense that we can use unsafe for "you've basically created a segfault with this API". I would instead expect this to be 80-90% fidelity (or something like that) and there are cases where you'd need to cheat a bit with types.

This doesn't seem to match with wasm-bindgen's existing philosophy:

• We already statically type imported functions, instead of only returning JsValues and dynamically type checking them all over.

• We already make distinctions between functions that can throw and functions that can't, instead of catching every single time we call into JS and requiring that every imported function return Result<_, JsValue>.

If we aren't going to trust our input (webidl / proc-macro attributes) and the types described therein, then I don't think we should be returning typed values from imported functions nor trusting whether an imported function may throw or not. In this world where we don't trust JS types, we should apply the "be conservative in what you send, be liberal in what you accept" principal to all interactions with JS:

• For imported functions and methods, we could statically type parameters, but we could only receive Result<JsValue, JsValue>s as return types (either a normal return value or a thrown value).

• For exported functions, we would only accept JsValue parameters, but we could return more specific, constrained types.

FWIW, I don't think this would be a bad world to live in. But it is very different from what we are doing now.

If we ascribe Rust's unsafe to this sort of "required cheating" it feels to me like a misuse of unsafe in the sense that unsafe is used to demarcate issues that can be caught with 100% fidelity.

The whole concept of unsafe is a bit funky when living inside a safe runtime. I think it makes more sense to talk about leveraging types. I would be happy to compromise on unchecked_cast not being unsafe -- I don't think it really matters that much in the wasm context.

[alexcrichton]

Hm so I definitely don't mean to say that we should distrust all the webidl sources. Nor do I think it's a lost cause to attempt to add types here and there to the web. Mainly though what I'm trying to say is that we shouldn't be striving for perfection. That may be a bit of a nebulous concept in my head though, but to me unsafe fn unchecked_cast(...) is an example of "perfection" where we're declaring we got everything "right enough" where you should basically almost never have to use that sort of function.

I do think there's a lot of value in adding types somewhere in the ecosystem and I think putting it at the web-sys layer is pretty appropriate. I believe, though, that the apis should be pretty flexible in what they accept. The types there are sort of for what 95% of the API calls accept but the remaining 5%+ would be done with casts and such as necessary.

This feels similar to the standard library's File type for example in that the low-level APIs all just take numbers and pointers, but we add a type wrapped around that, File, which is stronger than it needs to be. It's what you need 90% of the time though so we have the IntoRawFd and FromRawFd traits as an escape hatch if necessary (the unsafe here is for weird other reasons).

I guess what I'm getting at is that a perhaps even more conservative implementation of this RFC (to the end of "expose all runtime capabilities") is something like:

#[wasm_bindgen]
extern {
    type Foo;
}

// becomes ...

struct Foo(JsValue);

impl AsRef<JsValue> for Foo { ... }
impl AsMut<JsValue> for Foo { ... }
impl From<JsValue> for Foo { ... }
impl AsRef<Foo> for JsValue { ... }
impl AsMut<Foo> for JsValue { ... }
impl From<Foo> for JsValue { ... }

That way you should be able to convert in every direction as well as making a super-general API take something like T: AsRef<JsValue>.

[Pauan]

I guess what I'm getting at is that a perhaps even more conservative implementation of this RFC (to the end of "expose all runtime capabilities") is something like:

That's basically what stdweb does. Here's how you define a new JS type with stdweb:

#[derive(Clone, PartialEq, Eq, ReferenceType)]
#[reference(instance_of = "RegExp")]
pub struct RegExp(Reference);

The above code gets expanded (by the derive macros) into something like this:

pub struct RegExp(Reference);

impl AsRef<Reference> for RegExp { ... }
impl From<RegExp> for Reference { ... }
impl TryFrom<Reference> for RegExp { ... }
impl TryFrom<Value> for RegExp { ... }
impl JsSerialize for RegExp { ... }
impl InstanceOf for RegExp { ... }

• The Value type is for all JS values.

• The Reference type is for JS objects only.

• The JsSerialize trait is for things which can be sent to JS.

• The InstanceOf trait adds automatic run-time instanceof checking when converting a JS value into Rust (to ensure correctness).

This works out well in practice with stdweb, so I agree with your idea of making everything generic.

---

Speaking of inheritance, stdweb solves that with two systems:

1. Method inheritance is handled with traits. As an example:

   1. The IHtmlElement trait contains all of the methods for HTMLElement.

   2. IHtmlElement inherits from the IElement trait which contains all the methods for Element.

   3. IElement inherits from the INode, IParentNode, and IChildNode traits (this "multiple inheritance" is required by WebIDL because it has mixins/interfaces).

2. In addition to the traits, there are also structs which contain the actual JS objects (for example there's the IHtmlElement trait and the HtmlElement struct, then the IElement trait and the Element struct, etc.)

   Object inheritance is handled with From and TryFrom. When you define a type, you can mark it as a sub-class of other types, like this:

   #[derive(Clone, Debug, PartialEq, Eq, ReferenceType)]
   #[reference(instance_of = "HTMLElement")]
   #[reference(subclass_of(EventTarget, Node, Element))]
   pub struct HtmlElement(Reference);
   
   // Implement the various traits for HtmlElement
   impl IEventTarget for HtmlElement {}
   impl INode for HtmlElement {}
   impl IElement for HtmlElement {}
   impl IHtmlElement for HtmlElement {}

   The important part is the subclass_of macro, which automatically generates these impls:

   impl From<HtmlElement> for EventTarget { ... }
   impl TryFrom<EventTarget> for HtmlElement { ... }
   
   impl From<HtmlElement> for Node { ... }
   impl TryFrom<Node> for HtmlElement { ... }
   
   impl From<HtmlElement> for Element { ... }
   impl TryFrom<Element> for HtmlElement { ... }

   As you can see, upcasting is handled with From (which always succeeds), whereas downcasting is handled with TryFrom (which can fail).

If you look at the methods for the traits, you'll see that they usually accept generic arguments (IHtmlElement, IElement, etc.)

In other words, they accept anything that implements those traits, so you rarely need to use From/TryFrom to do casting, which makes things a lot more ergonomic.

[fitzgen]

@alexcrichton

Hm so I definitely don't mean to say that we should distrust all the webidl
sources. Nor do I think it's a lost cause to attempt to add types here and
there to the web. Mainly though what I'm trying to say is that we shouldn't be
striving for perfection.

I think we are in agreement here.

That may be a bit of a nebulous concept in my head though, but to me unsafe fn
unchecked_cast(...) is an example of "perfection" where we're declaring we got
everything "right enough" where you should basically almost never have to use
that sort of function.

I think the web-sys crate's WebIDL-derived definitions will be "right enough"
95% of the time, and one will be able to mostly use static upcasting.

I am not against unchecked dynamic casts, and I am not against marking them safe
either.

impl From<JsValue> for Foo { ... }
impl AsRef<Foo> for JsValue { ... }
impl AsMut<Foo> for JsValue { ... }

However, I think that these particular implementations are a little too much of
a footgun and encourage bad habits.

If a static upcast can be used, then it should be preferred over an unchecked
dynamic cast.

If a static upcast cannot be used, then one should have a small road bump to
think about whether they are using a Web API correctly and whether they should
consider an instanceof check or not. But they should definitely still have the
option to say "trust me, I got this" and do an unchecked dynamic cast, and that
operation can be safe.

Ultimately, I just want these implementations to be of different traits that are
structurally identical but give users that pause and cause them to think about
what they are doing. They should also have documentation about potential
pitfalls.

// Unchecked (but still safe) conversions.
impl UncheckedFrom<JsValue> for Foo { ... }
impl UncheckedAsRef<Foo> for JsValue { ... }
impl UncheckedAsMut<Foo> for JsValue { ... }

// Checked conversions using `instanceof`
impl TryFrom<JsValue> for Foo { ... }
impl<'a> TryFrom<&'a JsValue> for &'a Foo { ... }
impl<'a> TryFrom<&'a mut JsValue> for &'a mut Foo { ... }

---

@Pauan

Here's how you define a new JS type with stdweb:

#[derive(Clone, PartialEq, Eq, ReferenceType)]
#[reference(instance_of = "RegExp")]
pub struct RegExp(Reference);

The above code gets expanded (by the derive macros) into something like this:

pub struct RegExp(Reference);

impl AsRef<Reference> for RegExp { ... }
impl From<RegExp> for Reference { ... }
impl TryFrom<Reference> for RegExp { ... }
impl TryFrom<Value> for RegExp { ... }
impl JsSerialize for RegExp { ... }
impl InstanceOf for RegExp { ... }

I'm assuming these TryFrom implementations are doing instanceof checks? This
seems a bit different from the unchecked casts that @alexcrichton is
proposing.

1. Method inheritance is handled with traits. As an example:

   [...]

2. In addition to the traits, there are also structs which contain the actual JS
   objects (for example there's the IHtmlElement trait and the HtmlElement
   struct, then the IElement trait and the Element struct, etc.)

I alluded to this in the original RFC text. I was hoping that we could do this
sort of thing as a follow up, and implement just the casting incrementally.

I do think this general approach is the best way to get nicest ergonomics we can
out of mechanically-generated bindings. (Hand-written libraries customized for
ergo in some specific context can always do better -- think something like
glium -- but I believe this is outside web-sys's responsibilities.)

[alexcrichton]

@fitzgen that sounds good to me yeah! It also looks suspiciously similar to the current RFC so I'm trying to reread my original response and the RFC to figure out the main differences... I think it basically just comes down to the details of how we expect these to be used and exactly how we're setting up the traits?

For example these traits probably aren't suitable for "use everywhere as the base abstraction". An example of that you can't necessarily take T: Downcast<Base = MyRelevantType> because it doesn't apply to sub-sub classes. Additionally the T: Upcast<Self> on Downcast seems a little suspect to me, and it also doesn't provide facilities for downcasting through multiple class chains w/o doing multiple calls I think?

Ok so to may be put my thoughts a bit more clearly, I think I'm worried that the RFC as-written isn't achieving the goal of "expose possibility, even if it's not the most ergonomic". Now I think it can with a few tweaks though! Perhaps something like:

// in wasm_bindgen crate

pub unsafe trait InstanceOf {
    fn is_instance_of(val: &JsValue) -> bool;
    fn unchecked_from_js(val: JsValue) -> Self;
    fn unchecked_from_js_ref(val: &JsValue) -> &Self;
    fn unchecked_from_js_mut(val: &mut JsValue) -> &mut Self;
}

impl JsValue {
    pub fn instanceof<T>(&self) -> bool {
        T::is_instance_of(self)
    }

    pub fn try_into<T>(self) -> Result<T, Self>
        where T: InstanceOf 
    {
        if self.instanceof::<T>() { Ok(self.unchecked_into()) } else { Err(self) }
    }

    pub fn try_ref<T>(&self) -> Option<&T> 
        where T: InstanceOf
    {
        if self.instanceof::<T>() { Some(self.unchecked_ref()) } else { None }
    }

    pub fn try_mut<T>(&mut self) -> Option<&mut T> 
        where T: InstanceOf
    {
        if self.instanceof::<T>() { Some(self.unchecked_mut()) } else { None }
    }

    pub fn unchecked_into<T>(self) -> T where T: InstanceOf {
        T::unchecked_from_js(self)
    }
    
    pub fn unchecked_ref<T>(&self) -> &T where T: InstanceOf {
        T::unchecked_from_js_ref(self)
    }

    pub fn unchecked_mut<T>(&mut self) -> &mut T where T: InstanceOf {
        T::unchecked_from_js_mut(self)
    }

    // pub fn from<T: Into<Self>>(t: T) -> JsValue { t.into() } // provided by the Rust prelude
    pub fn from_ref<T: AsRef<Self>>(t: &T) -> &JsValue { t.as_ref() }
    pub fn from_mut<T: AsMut<Self>>(t: &mut T) -> &mut JsValue { t.as_mut() }
}

// and then in your crate

#[wasm_bindgen]
type Foo;
#[wasm_bindgen(extends = Foo)]
type Bar;

// generates

impl InstanceOf for Foo {
    fn is_instance_of(val: &JsValue) -> bool { /* shell out to JS shim */ }
    fn unchecked_from_js(val: JsValue) -> Self { /* do the constructor */ }
    fn unchecked_from_js_ref(val: &JsValue) -> &Self { /* do the weird transmute */ }
    fn unchecked_from_js_mut(val: &mut JsValue) -> &mut Self { /* do the weird transmute */ }
}
impl From<Foo> for JsValue { ... }
impl AsRef<JsValue> for Foo { ... }
impl AsMut<JsValue> for Foo { ... }

impl InstanceOf for Bar {
    fn is_instance_of(val: &JsValue) -> bool { /* shell out to JS shim */ }
    fn unchecked_from_js(val: JsValue) -> Self { /* do the constructor */ }
    fn unchecked_from_js_ref(val: &JsValue) -> &Self { /* do the weird transmute */ }
    fn unchecked_from_js_mut(val: &mut JsValue) -> &mut Self { /* do the weird transmute */ }
}

impl From<Bar> for Foo { /* use unchecked casts in InstanceOf */ }
impl AsRef<Foo> for Bar { /* use unchecked casts in InstanceOf */ }
impl AsMut<Foo> for Bar { /* use unchecked casts in InstanceOf */ }
impl From<Bar> for JsValue { ... }
impl AsRef<JsValue> for Bar { ... }
impl AsMut<JsValue> for Bar { ... }

...

Hm so actually that's very similar to my earlier comment, but in writing it up I've realized a few things:

• It should be possible to have a zero-cost unchecked cast between any two types
• It should be possible to have a checked cast between any two types using at most one instanceof
• It should be possible to upcast up an inheritance chain in a zero-cost fashion with no "unchecked" operations

I'm curious if you agree w/ these constraints? I think it fits what we've been saying here pretty well so far, but want to make sure I didn't miss something!

Now on a totally unrelated note from functionality when we get to API-design business I'm personally wary to lean too heavily on traits. I find they can often serve as a barrier to understanding because you're always feeling like you should be using a trait somewhere or are otherwise too aggressively implementing traits on things that aren't used all that often (or bemoaning a missing blanket impl or lack of specialization or something like that). The ergonomics also aren't too great as you have to import them, but wasm_bindgen's prelude can solve this. In any case I liked the idea of using standard Rust prelude traits (if we can) and otherwise discouraging use of the trait here (InstanceOf) through APIs on JsValue which are slightly more easy to discover/document. This isn't related to the functionality of the API though! Just the exact construction of what's a trait and what isn't.

[fitzgen]

• It should be possible to have a zero-cost unchecked cast between any two types
• It should be possible to have a checked cast between any two types using at most one instanceof
• It should be possible to upcast up an inheritance chain in a zero-cost fashion with no "unchecked" operations

Yes! This is the crux of it.

[fitzgen]

The one question still unanswered is whether we will generate As{Ref,Mut} and From impls for the whole inheritance chain, or just one level deep (and requiring users to chain .as_ref()s).

#[wasm_bindgen]
extern {
    type Base;

    #[wasm_bindgen(extends = Base)]
    type Derived;

    #[wasm_bindgen(extends = Derived)]
    type DoubleDerived;
}

I think for the proc-macro we can only do one level due to technical reasons (for example if the definitions are split across extern blocks).

For WebIDL we could possibly do the whole thing.

[alexcrichton]

I think for the proc-macro we can only do one level due to technical reasons

Agreed!

For WebIDL we could possibly do the whole thing.

Also agreed!

To bridge the gap we could perhaps allow multiple extends attributes for handwritten bindings?

[fitzgen]

To bridge the gap we could perhaps allow multiple extends attributes for handwritten bindings?

SGTM!

Will update the RFC text tomorrow.

[fitzgen]

RFC text updated!

@alexcrichton @icefoxen @ohanar what do y'all think?

[fitzgen]

@Pauan interested in your feedback on the updated RFC as well! Sorry I forgot to tag you in that last comment :-p

[alexcrichton]

Looks great to me, thanks for updating @fitzgen!

Do we need a split between the two traits still? I think it'd be possible to have only one trait for these operations, right?

[fitzgen]

Do we need a split between the two traits still? I think it'd be possible to have only one trait for these operations, right?

This would require implementing InstanceOf for JsValue, which we could do but also makes the InstanceOf trait not map directly onto the instanceof operator since there is no constructor for js values to use with instanceof and it is trivially true that all JS values are instances of JS value.

Thoughts?

[alexcrichton]

Oh I think that's fine, it just means that the "instanceof" check there always returns true in that everything's a instance of JsValue, which seems fine by me!

[ohanar]

Overall, I'm pretty happy with the proposal, there are just a couple small comments I have:

• It is a little annoying right now that upcasting with a turbo-fish requires using unchecked_*. This might not be too big of an issue after we decide on a more ergonomic way to expose base methods.
• I would prefer avoiding using the name try_into in the JsCast trait. I would like to leave open the future possibility of implementing the standard fallible conversion traits, and it would be unfortunate to have a name collision.

[Pauan]

@fitzgen I'm assuming these TryFrom implementations are doing instanceof checks?

Yes, the TryFrom implementations use instanceof. To be more specific, the TryFrom implementation calls the downcast method, which in turn calls the instance_of method (which is automatically created by #[reference(instance_of = "Foo")]). All of this is automated by the macros, so users don't need to deal with it.

On the other hand, the From implementations don't use instanceof (they are zero-cost and unchecked).

And it's always possible to use from_reference_unchecked to do an unsafe (but zero-cost) cast between different types.

I alluded to this in the original RFC text. I was hoping that we could do this sort of thing as a follow up, and implement just the casting incrementally.

Sure, I did see that. But since stdweb has already tackled this problem, I wanted to explain the solution they came up with, since it acts as evidence that the general idea presented in this RFC is both sound and practical.

---

interested in your feedback on the updated RFC as well! Sorry I forgot to tag you in that last comment :-p

Sure! I'll reply individually to parts of the RFC:

Foundation: The InstanceOf Trait

We should consider marking the unchecked methods as unsafe (even though they probably can't cause Rust unsafety, they still shouldn't be easy to use!)

The JsCast Trait

Why the separation between InstanceOf and JsCast?

Only implement its methods on JsValue and require that conversions like ImportedJsClassUno -> ImportedJsClassDos go to JsValue in between: ImportedJsClassUno -> JsValue -> ImpiortedJsClassDos.

I'm fine with this. In almost every case there will be From and TryFrom (or whatever) implementations (which are auto-generated by #[wasm_bindgen(extends = Foo)]), so that sort of manual conversion (with JsValue in-between) should be extremely rare.

Should the JsCast trait be re-exported in wasm_bindgen::prelude?

I view manual casting as a very advanced thing which should rarely be needed: users should use From / TryFrom instead. So I don't think JsCast should be exported in the prelude.

[fabricedesre]

One use case I have is to implement Custom Elements in Rust. In this situation, the Rust struct needs to inherit from (for instance) HTMLElement and implement the custom elements lifecycle callbacks (see details at https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements#Using_the_lifecycle_callbacks).

It's unclear to me how this RFC would allow that since it seems focused on exposing the JS inheritance chain to Rust when importing JS but not when exporting.

[alexcrichton]

@fabricedesre I think that's covered by rustwasm/wasm-bindgen#210 which isn't addressed by this RFC, although the components in this RFC should be hopefully adding support for it in the future!

[fitzgen]

New commit merged InstanceOf into the JsCast trait, so there is only one trait here.

We implement JsCast for JsValue with trivial no-ops and identity functions.

Renamed JsCast::try_{into,ref,mut} to JsCast::dyn_{into,ref,mut} so that we are future compatible with TryFrom and TryInto.

[fitzgen]

I'd like to now propose that we enter the Final Comment Period for this RFC.

Aside: we might want to eventually make a bindings/lib/something team that handles RFCs that are related to wasm-bindgen's output and design, but since such a team does not exist at the moment, I will ask @rustwasm/core to sign off.

Disposition: merge

@rustwasm/core members to sign off:

• @fitzgen
• @ashleygwilliams
• @alexcrichton

[fitzgen]

All @rustwasm/core team members have signed off. Entering the 7 day ✨ Final Comment Period ✨!

[alexcrichton]

A sample implementation of the current state of this RFC is at rustwasm/wasm-bindgen#640

[fitzgen]

FCP complete! Thanks everyone for the discussion :) I think we ended up in a better place than where we started!