WebAssembly / interface-types

Other
641 stars 57 forks source link

Statically rule out "weird" incoming binding expression #37

Closed alexcrichton closed 5 years ago

alexcrichton commented 5 years ago

In a recent large refactoring of wasm-bindgen I ended up writing what amounts to translating an incoming webidl binding expression to a JS shim. In doing so I found that some incoming binding expressions don't really make much sense, for example:

(alloc-utf8-str "malloc" (alloc-utf8-str "malloc" (get 1)))

or things like:

(field "foo" (as i32 (get 1)))

I was wondering if perhaps it'd be better to split incoming binding expression into two forms of expressions? One form of expression would be actually acquiring a value, and the second would be converting that value to WebAssembly. The idea here would be that while there's some bindings that are tree-like in reality each binding at the top level is a conversion from a value to wasm values, but once you get wasm values it seems odd to do more transformations that are otherwise expecting WebIDL-like values.

A strawman proposal for this would be:

in-value-expr

Operator Immediates Children
get idx
field field‑idx in‑value-expr

in-expr

Operator Immediates Children
as wasm‑type in‑value-expr Take the result of in-expr, which can be any Web IDL value, and produces a wasm value of wasm-type, allowing only trivial conversions (like [long] to i32 or [any] to anyref).
alloc‑utf8‑str alloc‑func‑name in‑value-expr
alloc‑copy alloc‑func‑name in‑value-expr
enum‑to‑i32 webidl‑type in‑value-expr
bind‑import wasm‑type
binding
in‑value-expr
fgmccabe commented 5 years ago

This structuring was already in mind.

On Fri, Jun 14, 2019 at 1:09 PM Alex Crichton notifications@github.com wrote:

In a recent large refactoring https://github.com/rustwasm/wasm-bindgen/pull/1594 of wasm-bindgen I ended up writing what amounts to translating an incoming webidl binding expression to a JS shim. In doing so I found that some incoming binding expressions don't really make much sense, for example:

(alloc-utf8-str "malloc" (alloc-utf8-str "malloc" (get 1)))

or things like:

(field "foo" (as i32 (get 1)))

I was wondering if perhaps it'd be better to split incoming binding expression into two forms of expressions? One form of expression would be actually acquiring a value, and the second would be converting that value to WebAssembly. The idea here would be that while there's some bindings that are tree-like in reality each binding at the top level is a conversion from a value to wasm values, but once you get wasm values it seems odd to do more transformations that are otherwise expecting WebIDL-like values.

A strawman proposal for this would be: in-value-expr Operator Immediates Children get idx field field‑idx in‑value-expr in-expr Operator Immediates Children as wasm‑type in‑value-expr alloc‑utf8‑str alloc‑func‑name in‑value-expr alloc‑copy alloc‑func‑name in‑value-expr enum‑to‑i32 webidl‑type in‑value-expr bind‑import wasm‑type binding in‑value-expr

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/WebAssembly/webidl-bindings/issues/37?email_source=notifications&email_token=AAQAXUHLUXNHSI4NFTF6TEDP2P3IHA5CNFSM4HYMZSJKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GZUKXWA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQAXUHF2ROMOEIFZP3IIJLP2P3IHANCNFSM4HYMZSJA .

-- Francis McCabe SWE

lukewagner commented 5 years ago

Yup, me too; I think this is the role of the type system in validating binding expressions.

alexcrichton commented 5 years ago

Oh sorry I should have clarified that my intention here wasn't necessarily to add more typechecking but moreso not rely on typechecking for this sort of validation, but rather the staitc structure. I don't think it's possible to typecheck where get or field is a top-level expression, nor can any of the other in-expr above show up as an input to another in-expr, so I was curious to just reflect this in the static structure of the section as opposed to relying on typechecking to figure it out.

fgmccabe commented 5 years ago

It seems to me that you must rely on type checking. But then, personally I am not afraid of type checkers :)\

On Tue, Jun 18, 2019 at 9:42 AM Alex Crichton notifications@github.com wrote:

Oh sorry I should have clarified that my intention here wasn't necessarily to add more typechecking but moreso not rely on typechecking for this sort of validation, but rather the staitc structure. I don't think it's possible to typecheck where get or field is a top-level expression, nor can any of the other in-expr above show up as an input to another in-expr, so I was curious to just reflect this in the static structure of the section as opposed to relying on typechecking to figure it out.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/WebAssembly/webidl-bindings/issues/37?email_source=notifications&email_token=AAQAXUGDUTMVGQLJKSCFSODP3EF6DA5CNFSM4HYMZSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX7HSGA#issuecomment-503216408, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQAXUFGU6C64NXAV5EWDJTP3EF6DANCNFSM4HYMZSJA .

-- Francis McCabe SWE

fitzgen commented 5 years ago

I am also in favor of pushing static constraints from the (so far, unspecified) validation algorithm, to the grammar itself, when possible. It means that you we can do less validation work after parsing, since if something parsed and the grammar enforces a certain property, then we know that the relevant property is upheld without doing another validation pass.

This is obviously not possible for every property, but in the case of the straw proposal in the OP, it is possible and makes sense to me.

pchickey commented 5 years ago

Closing as out-of-date: the proposal has evolved and no longer involves binding expressions as described here. A validation algorithm still needs to be specified, see #29