WebAssembly / reference-types

Proposal for adding basic reference types (anyref)
https://webassembly.github.io/reference-types/
Other
162 stars 40 forks source link

[js-api] Formalization only: eliminate extern value cache #101

Closed RossTate closed 4 years ago

RossTate commented 4 years ago

Nothing in here is meant to change the behavior of the proposal. It is just a suggestion on how to formalize the existing behavior.

The extern value cache doesn't seem to serve a purpose.

One purpose I can imagine is as a formalization device: it associates a natural number that represents a JS object. But that purpose seems addressible by letting externaddr in the core spec be an abstract set of values specified by the embedder, in which case ref.extern could take a JS object as its argument. (That's a matter of taste though, so I'm happy to leave it as is.)

But the extern value cache also seems to be ensuring that every JS object has at most one natural number associated with it as well as determinizing what that number is. Since this number has no way of leaking into either wasm or JS (or at least I would think it shouldn't), this seems unnecessary.

binji commented 4 years ago

cc @littledan

littledan commented 4 years ago

Editorial changes are welcome, if you can build consensus with the core spec maintainers. I think things work fine the way they are, and expect that people may debate whether things are clearer or less clear with this kind of change.

One purpose I can imagine is as a formalization device: it associates a natural number that represents a JS object.

Yes, it does seem to be doing this.

But that purpose seems addressible by letting externaddr in the core spec be an abstract set of values specified by the embedder, in which case ref.extern could take a JS object as its argument.

I wouldn't mind this change, but my worry would be that this complication of the core spec could be unwelcome. That's why these caches exist for the other types, anyway.

RossTate commented 4 years ago

Thanks, @littledan. It sounds like at least my suspicion that the extra steps are not necessary is correct, and that they seem easy to fix (an editorial change, as you say). The other types in the spec are a little different because they interact with wasm instructions. But externref is completely uninterpreted, besides null, and so might be easy enough to treat more abstractly in the formalism. It sounds like we should have @rossberg let us know his preference, then we can determine next steps appropriately.

gahaas commented 4 years ago

One advantage of having the extern value cache is its consistency with the other caches.

rossberg commented 4 years ago

As @gahaas points out, the appeal of the current solution is consistency between different types of references, both in the core spec and the JS API. Since we'll need caches anyway, it's not a structural simplification to create a special case for one kind. It would only obscure the commonalities.

One other thing to consider is that we'll want to change the representation of addresses with the threads proposal (to better account for non-deterministic allocation order). So I'd rather not make conflicting changes for now.

littledan commented 4 years ago

@RossTate Yes, there are always other ways to phrase things. Ultimately, I think all the caches exist for the same reason: they could all be replaced by threading a "host pointer" through the core spec.

RossTate commented 4 years ago

Okay, makes sense given that it sounds like there will be a pass made in the threads proposal to remove various unnecessary non-determinism in the spec. Thanks!