Add RFC for Deref in web-sys

[alexcrichton]

This proposal is a formal alternative to RFC 3 which has evolved over time through the comments, hopefully now all written down in one place!

Rendered

[fitzgen]

This looks great to me, and I think it is the right set of trade offs given the choices we have.

[Pauan]

Overall this looks great, thanks! I generally approve, but I listed a few minor comments.

[Pauan]

This seems like a footgun. Perhaps it could be split into two separate attributes, like this?

#[wasm_bindgen(parent = Node, extends = Object)]

So that way it's crystal clear what's going on, and it's harder to make mistakes (and if somebody does make a mistake, such as using extends without parent, it can print a nice error message).

[alexcrichton]

Yes I'd be fine adding that in the future, but for now allowing multiple extends while still implementing Deref is a backwards-compatibility requirement.

[alexcrichton]

I also think long term we don't want a new attribute, but rather only accepting one extends attribute.

(and maybe adding a special attribute for generated AsRef impls)

[Pauan]

@alexcrichton Is it a backwards compatibility requirement? The RFC itself says that web-sys will have to be changed so that it always puts the immediate parent first, so this requirement requires code changes. The same is true for non-web-sys bindings as well.

So I don't see how this enables backwards compatibility. In fact, it may even be a huge footgun: some existing code might put the wrong class first, but this doesn't give a warning or error, so they don't realize it.

If we instead required a new parent attribute, we would give a nice error message if it's missing, and this lets people know that they need to change their code so that the immediate parent is correct.

[alexcrichton]

Yes this would otherwise be a breaking change for wasm-bindgen, which we're not doing at this time. web-sys can be updated easily, other crates cannot.

[Pauan]

Somehow that still feels wrong. If there exists code where the first extends isn't the immediate parent, then they will now get an automatic Deref impl which is wrong (or at least unexpected).

We could instead tie Deref to the parent attribute, so that if code wants the Deref impl they have to change to use parent (existing code without parent will keep the old behavior and not have a Deref impl).

[Pauan]

Technically this is a breaking change, since somebody might be relying upon the non-structural behavior.

I think that's very unlikely though, so I'm personally fine with not treating it as a breaking change.

[Pauan]

I don't like the name final, because final means "this method cannot be overridden", but we're using it to mean "this method can be overridden, but if it is overridden it will have subtly wrong behavior".

Maybe prototype is a good alternative? Or perhaps static_prototype to be even more specific.

[michiel-de-muynck]

we're using it to mean "this method can be overridden, but if it is overridden it will have subtly wrong behavior"

Well, I'd argue we're using it to mean "this method should not be overridden". Yes, there is a small distinction between that and "this method can not be overridden", in that we can't prevent the method from being overridden, but I don't think people will assume that we somehow can prevent it, since we're only providing bindings. So I don't really see the issue.

Either way, I think the distinction between the two meanings is small enough that we should reuse the well-known name "final" rather than trying to invent a completely new name for it.

[alexcrichton]

@Pauan I think "subtly wrong behavior" is too strong here, but rather final indicates that this method, like final indicates, will not be overridden and regardless of class passed in it'll call this precise method defined for this precise class. That's "subtly wrong" when applied incorrectly but it can also be applied correctly in which case it's not "subtly wrong".

In that sense I agree with @migi, the name final invokes thoughts about the right concept, namely that calling this method will always call what's being defined.

[Pauan]

@migi but I don't think people will assume that we somehow can prevent it, since we're only providing bindings.

You would be surprised, especially when we're using well-established OOP terms.

---

@alexcrichton I think "subtly wrong behavior" is too strong here

"subtly wrong behavior" is correct. We're talking about the user expecting it to call Bar::some_method but instead it calls Foo::some_method. What would you call that if not "subtly wrong behavior"?

but rather final indicates that this method, like final indicates, will not be overridden

That's my point: with final in wasm-bindgen, you can override the method. And this isn't hard to do either: it's possible for class hierarchies to be spread across multiple Rust crates, meaning that a downstream crate might override some methods on an upstream crate (unaware that they shouldn't do this, because of final).

That's "subtly wrong" when applied incorrectly but it can also be applied correctly in which case it's not "subtly wrong".

That's why I specifically said if the method is overridden it will have subtly wrong behavior.

[alexcrichton]

@Pauan I would not call it "subtly wrong behavior" because it simply is not. You'd have to go out of your way to write down final which means you probably know what you're doing, so saying that you intended a subclass method I think is wrong because a manual annotation was added to say "I don't want this behavior".

If the final modifier is applied blindly without understanding it then that results in the same misunderstanding from blindly applying anything without understanding it.

Also, no, you cannot override final. When you say final you're saying you're calling precisely the method indicated, no others. When calling the final import function you'll always call that one, no overridden function. We're not defining JS classes in Rust so we have no control over how the JS hierarchy looks in JS, but the bindings are for the Rust hierarchy and defining what's happening in Rust.

[Pauan]

Also, no, you cannot override final. When you say final you're saying you're calling precisely the method indicated, no others. When calling the final import function you'll always call that one, no overridden function

I am aware of that. You are right that it will always call Foo.prototype.someMethod, but that's my point: that might cause subtle (and not so subtle!) problems. As an example, consider this code:

class Foo {
    constructor() {
        this.someProperty = "hi";
    }

    someMethod() {
        return this.someProperty.slice(0, 0);
    }
}

class Bar extends Foo {
    constructor() {
        super();
        this.someProperty = null;
    }

    someMethod() {
        return "";
    }
}

new Bar().someMethod()                    // returns ""
Foo.prototype.someMethod.call(new Bar())  // errors

This is obviously contrived, but I'm sure similar situations do happen out in the wild.

In this case Bar extends Foo, so it has the same internal state (plus maybe some additional state). It can change its own state in such a way that Foo.prototype.someMethod is now broken/wrong (but Bar.prototype.someMethod is correct).

There's plenty of other situations as well: such as some code relying upon the fact that Bar.prototype.someMethod is called (because it updates the internal state of Bar), so calling Foo.prototype.someMethod causes weird/broken behavior.

You might say "oh, but then you shouldn't use final", and you're absolutely right. It's easy to see that in isolation, but I'm talking about a situation like this:

1. Somebody defines a rust-foo Rust crate which creates bindings for the npm-foo package.

   Since they're not overriding any methods on the classes (and perhaps due to some confusion with how final works in OOP languages), they mark the methods as final. Looking at the rust-foo crate in isolation, this seems okay (since no methods are being overridden):

   #[wasm_bindgen(module = "npm-foo")]
   extern {
       type Foo;
   
       #[wasm_bindgen(method, final)]
       fn some_method(this: &Foo);
   }

2. Somebody else creates a rust-bar Rust crate which creates bindings for the npm-bar package. The npm-bar package has some classes which extend the classes defined in npm-foo, and so likewise the rust-bar crate puts in some extends attributes (which point to the bindings created in rust-foo):

   #[wasm_bindgen(module = "npm-bar")]
   extern {
       #[wasm_bindgen(extends = Foo)]
       type Bar;
   }

3. The classes in npm-bar override some methods, but the creator of the rust-bar crate is unaware of this (and even if they were aware, they cannot control the rust-foo crate, so they can't remove the final attribute).

Basically, it's a situation where in isolation final seems reasonable, but it ends up being wrong in practice.

Personally, I feel that the semantics of final in wasm-bindgen are different enough (and the use case is niche enough) to warrant a different name than final. But I'm not going to fight you on that.

In any case, the docs will need to be very clear about the following points:

• Exactly what final does (and how it differs from final in OOP languages, in particular it does not prevent overriding the method).

• You almost never need final (it's strongly preferred to not use it).

• You should only use final if you can guarantee that the method will not be overridden by you or by anybody else.

[michiel-de-muynck]

I agree about those last 3 bullet points: the docs should indeed say those things clearly.

But here's the thing: To misunderstand the final keyword in the way you fear would happen, you basically have to not have read the docs on final at all. To think that final prevents overriding is a very different meaning from what final actually does. But since the author of the rust-foo binding has to explicitly write that final attribute, I think it's fair to assume that he would have read the docs, at least partially.

[michiel-de-muynck]

I mean if we really want to emphasize the fact that you should be careful when marking a function "final" we could use the name unsafe-final, but that's putting a lot of emphasis on an overall pretty minor thing.

[Pauan]

Is there a reason this is marked as structural even though above it doesn't use structural?

[alexcrichton]

This was a mistake

[Pauan]

Thanks, this is good data. It also aligns with my personal beliefs about JS engines: because structural is much more common in JS than final, it makes sense that the JS engines heavily optimize for structural.

[Pauan]

Oh, by the way, it might be worth pointing out that web-sys can continue to use final methods, since it's known that web-sys methods aren't overridden.

So regardless of what the default is, it won't affect web-sys, so the default is for the sake of non-web-sys APIs (which are generally created by hand).

[michiel-de-muynck]

Excellent write-up. It explains the situation as I see it very well. I agree with essentially all of it.

I'm strongly in favor of making methods structural by default. final is at best a minor optimization but at worst a giant foot-gun with a silencer.

I'm weakly in favor of Deref versus traits. If we go with structural unless explicitly marked otherwise, then both solutions do the right thing. The trade-offs between the two are pretty minor and mostly subjective, and I could live with either solution.

[alexcrichton]

@Pauan ah yes, that was my intention but I forgot to explicitly say it. When final is faster we can switch everything in web-sys transparently.

[michiel-de-muynck]

An article was just posted that is very relevant to the benchmarks in this RFC: Calls between JavaScript and WebAssembly are finally fast 🎉.

A brief summary: Firefox just implemented some optimizations that allows wasm to call JIT-ed JavaScript directly, and vice versa, without going through a C++ "trampoline". The optimizations made wasm->JS calls about 2x faster, and both JS->wasm and wasm->built-ins about 10x faster.

It may be worth re-running the Firefox benchmark using the latest Firefox nightly to see if these optimizations dramatically affect the performance trade-off between structural and final.

[alexcrichton]

A good point! Some testing shows, however, that there's not much difference. On today's nightly Firefox build structural is 5% faster, roughly where it was before. As the RFC mentions though the benchmark is available publicly for anyone to tinker with it

[fitzgen]

Bikeshedding the attribute name:

• static_dispatch
• statically_dispatched
• no_proto_walk

[alexcrichton]

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

Disposition: merge

@rustwasm/core members to sign off:

• @fitzgen
• @ashleygwilliams
• @alexcrichton

I think the remaining issue of attribute names can be figured out during FCP, but if changing I'd lean towards the name of no_proto_walk or something like that.

[fitzgen]

I am in favor of merging this RFC, but I (also) think it is worth bike shedding the name of the non-structural attribute a little more (and am also happy to do this in FCP time).

---

final in C++ (I don't know about other bracket OO languages: Java, C#) really means it is impossible to override the method. Where as what we are dealing with here is a situation where it is still possible for other JS classes to override the method, but our bindings won't ever call the overridden method. I think the "final" terminology has the potential to confuse folks coming from a C++ background (and maybe C# and Java?). I don't think we should bend over backwards for this group, since I don't think it is either of our core audiences of Rust programmers targeting the Web and Web programmers learning Rust. However, it would be easy to choose another name in this case, so renaming seems like a win/win.

In C++ land, structural is like virtual methods, so how about we call this non_virtual?

Also happy with no_proto_walk since that accurately describes exactly what is not happening and doesn't come with preconceived baggage from other languages.

[Pauan]

Since the semantics of JS are weird (prototypal inheritance, etc.) I'm in favor of a JS-specific name.

Many good options have been suggested, I don't have a strong opinion about them, but I do like static_dispatch or static_prototype_dispatch.

Since it should be used rarely, it's okay if the name is weird, long, and unwieldy.

[michiel-de-muynck]

I'm also in favor of this RFC. At least the technical aspects of it. As far as the name of the attribute goes, I don't care very much one way or the other (it's just a name after all), but I do want to present a few more points in favor of final:

final is used to mark imported functions, not exported functions. When you mark an imported type or function with any attribute, that always means that it's a description of what that type/function is/does on the JavaScript side. There is not a single attribute on an imported item that somehow changes the behavior of that type or function on the JavaScript side. That would be weird. Only attributes on exported items change the behavior of things on the JavaScript side. And I think people intuitively understand this.

For example, if a JavaScript class extends another one, you mark the imported type as extends = .... Marking the imported type with extends = Foo does not somehow force the class to extend Foo in JavaScript. It merely describes that you believe that the class extends Foo. I haven't heard of anyone being confused about this and mistakenly thinking that the extends attribute on an imported type somehow forces the JavaScript class to extend the given class, so why would they think that marking a function final somehow forces the function to become non-overridable on the JavaScript side?

Say (hypothetically) that a future EcmaScript version would add "final" as a keyword to mark methods as non-overridable (they almost did, in the (abandoned) ES4 proposal). Then any method that is final in JavaScript, you would mark final in your Rust bindings, which is very consistent with how all the other attributes on imported types/functions work:

• An imported class extends another? You mark it as extends = ....
• An imported function is a constructor? You mark it as constructor.
• An imported method is a getter? You mark it as getter.
• An imported method is final? You mark it as final.

[alexcrichton]

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

[alexcrichton]

Ok! That concludes the 7-day FCP, and I've gone ahead and merged this. It does seem like the naming is still somewhat up in the air, but I figure it's ok to go ahead and start getting to the implementation

[alexcrichton]

For those interested I have an implementation of this at rustwasm/wasm-bindgen#1019 where I've proposed renaming final to host_binding