Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge Module Linking and Interface Types proposals #6

Merged
merged 19 commits into from
Mar 18, 2022
Merged

Conversation

lukewagner
Copy link
Member

This PR merges together and simplifies the Module Linking and Interface Types proposals into one explainer and binary format doc that can be read top-to-bottom without needing to mentally glue together two repos. The original reason for having two repos is that Module Linking was a normal Core WebAssembly proposal while Interface Types was its own thing. However, after the layering and scoping decision, these were to become two proposals making up a single new layered spec in a single MVP release. After working on this for a while, I found that, similar to our previous experience with reference-types+bulk-memory, this split was causing more harm than good, so this PR merges the two and also simultaneously moves active development over to this component-model repo. In particular, with this merge, it's natural to punt this intermediate "adapter module" concept out of the MVP which is a significant simplification (see also FutureFeatures.md in this PR).

For now, I'd like to temporarily halt work in the module-linking and interface-types repos, but in the future they may come back to life:

  • If there is renewed interest in adding the declarative style of module linking to Core WebAssembly, as was originally proposed, we could easily revert the module-linking repo to original Core proposal and move forward with that.
  • Post-component-model-MVP, interface-types would be the natural repo to (continue) discussing custom adapter functions.

This PR also fills in a bunch of details for how interface types and canonical adapters fit into components which has been pending since the original layering decision. Unfortunately, the canonical ABI itself is still left as a "TODO" in the CanonicalABI.md in this PR; in parallel with getting this PR reviewed, I'll work on updating and transforming Alex's PR in interface-types/#140.

cc @alexcrichton @peterhuene @fitzgen @sunfishcode @rossberg @Pyrolistical @guybedford

Lastly, the `(instance <export>*)` and `(instance <core:export>+)`
expressions allow component and module instances to be created by directly
tupling together preceding definitions, without the need to `instantiate`
anything. To disambiguate the empty case, we observe that there is never
Copy link
Member

@sunfishcode sunfishcode Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saying "without the need to instantiate anything" is a little confusing; as I understand it, the tupling form is effectively sugar for creating a module/component which just imports and re-exports things, and then does an instantiate on that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that the net effect is achievable via helper-component, but as I've started digging into the semantics, I don't think it will actually be defined by desugaring. Rather, I've come to understand instance types as the types of instantiation-time record values (which, as values, are immutable and without identity) and instantiate as an instruction that consumes and produces these values. This "instantiation-time record value" concept is subtly different than what I've traditionally meant by "instance" which is this stateful persistent runtime object created by instantiate, but I think it's a crucial distinction when you consider the case of merging the exports of several instances into a new instance. The distinction will also become more concrete as we dig into the consumed-exactly-once validation rules for value (and instances containing them...). Getting back to this comment, though: with the understanding of "instance as a record value", this tupling (instance <export>*) form is just a primitive value constructor.

design/mvp/Explainer.md Outdated Show resolved Hide resolved
design/mvp/Explainer.md Outdated Show resolved Hide resolved
design/mvp/Binary.md Outdated Show resolved Hide resolved
design/mvp/Binary.md Outdated Show resolved Hide resolved
design/mvp/Explainer.md Outdated Show resolved Hide resolved
Co-authored-by: Dave Bakker <github@davebakker.io>
design/mvp/Binary.md Outdated Show resolved Hide resolved
design/mvp/Explainer.md Outdated Show resolved Hide resolved
@badeend
Copy link
Contributor

badeend commented Feb 26, 2022

This PR introduces some renamings:

  • f32 -> float32
  • f64 -> float64
  • option -> optional

Were these intentional?

@lukewagner
Copy link
Member Author

@badeend The f32/f64float32/float64 renaming reflects the subtle semantic change (regarding NaN) proposed in interface-types/#134, so that was quite intentional.

But for optional, I had actually confused myself by working with too many locally-modified drafts into thinking that optional was the original name. But I see now it's option in the current interface-types explainer and in wit-bindgen today, so I'll revert to option. Thanks for double-checking that!

@badeend
Copy link
Contributor

badeend commented Feb 26, 2022

But for optional, I had actually confused myself by working with too many locally-modified drafts into thinking that optional was the original name. But I see now it's option in the current interface-types explainer and in wit-bindgen today, so I'll revert to option. Thanks for double-checking that!

👍

The f32/f64 → float32/float64 renaming reflects the subtle semantic change (regarding NaN) proposed in WebAssembly/interface-types#134, so that was quite intentional.

I understand. But TBH, I think that without knowing this background information most people will assume f64 to be an alias of float64. If the distinction between canonicalized and non-canonicalized floats is really that important, maybe that should be reflected in the name, like: f64-canon (instead of float64)

@lukewagner
Copy link
Member Author

If the distinction between canonicalized and non-canonicalized floats is really that important, maybe that should be reflected in the name

Since almost all folks don't need or want to know about the super-corner-case of non-canonical nan payloads and since the semantics of float64 matches the usual assumption that there is a single NaN value and since the intended audience of interface types is folks who (eventually, once the toolchain stabilizes) shouldn't need to know any low-level details about core wasm and f64 (in the same way that you don't need to know x64 to use Go running on x64), I think this is a subtly that is best left out of the name.

design/mvp/Subtyping.md Outdated Show resolved Hide resolved
design/mvp/Subtyping.md Outdated Show resolved Hide resolved
design/mvp/Explainer.md Outdated Show resolved Hide resolved
(flags <name>*) ↦ (record (field <name> bool)*)
(enum <name>*) ↦ (variant (case <name> unit)*)
(option <intertype>) ↦ (variant (case "none") (case "some" <intertype>))
(union <intertype>*) ↦ (variant (case "𝒊" <intertype>)*) for 𝒊=0,1,...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Union" is a bit overloaded, without qualification it usually refers to a non-disjoint overlay of types (with more general subtyping rules). As that is not the case here, this could lead to some confusion. Unfortunately, I don't have a good suggestion for an alternative name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I suppose there is some divergence here, but I can't think of a good alternative either. It's nice that it lines up with Web IDL (and thus the JS API) and that misunderstandings (trying to add non-disjoint cases) will be a relatively early error.

interface types is given by the following table:
| Type | Values |
| ------------------------- | ------ |
| `unit` | just one [uninteresting value] |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might also be worth introducing the dual vacuous type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, I wondered about this too. My worry is that this will end up introducing more implementation work and special-casing than the value it provides (symmetry and describing non-return), so I'm inclined to wait until someone specifically asks for it with a concrete benefit in mind.

Co-authored-by: Liam Murphy <liampm32@gmail.com>
instancetype-def ::= <type>
| <alias>
| (export <name> <deftype>)
functype ::= (func <id>? (param <name>? <intertype>)* (result <intertype>))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some of the examples, the (result <intertype>) is missing, such as in (export "log" (func (param string))). Should the result part be optional, or should the examples add a result?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch! In the abstract type, there is always a single result type, which can be unit. Surface text formats like wit would map -> [] to -> unit, so I suppose the text format could do likewise as syntactic sugar (so a missing result was the same as (result unit)).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kindof like being able to leave off result and the overhead as a pure text format sugar seems low, so I tentatively did that. To avoid confusing things too much (given that the explainer merges the text format with AST), I left (result <intertype>) non-optional in the AST grammar and just added a note regarding the optionality sugar.

@lukewagner
Copy link
Member Author

It looks like new comments are winding down, so if it stays that way for a week, I think I'll merge this PR, with the usual understanding that this isn't final and we can iterate in future issues/PRs. (In the meantime, I'm working on importing and updating the Canonical ABI as the next PR.)

@lukewagner lukewagner merged commit 42ae298 into main Mar 18, 2022
@lukewagner lukewagner deleted the ast-and-binary branch March 18, 2022 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants