This repository was archived by the owner on Aug 17, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 13
Reframe Module Linking as the first proposal of the Component Model #35
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexcrichton
approved these changes
Nov 5, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything here looks good to me! I may have more comments in the future as we get to implementing this, but overall reading through the binary/explainer documents everything is as expected.
4d05078
to
82c9722
Compare
82c9722
to
36c476b
Compare
rossberg
reviewed
Nov 11, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great write-up, very easy to consume!
@rossberg Thanks a bunch for the review! I addressed everything except as noted above. PTAL |
rossberg
approved these changes
Nov 16, 2021
Ok, I'll go ahead and merge and happy to continue iterating via the usual issues and PRs. Thanks again Alex and Andreas. |
This was referenced Nov 16, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR keeps all the same concepts of the Module Linking proposal, but rewrites the entire proposal to be the start of a new spec (the Component Model) layered on top of Core WebAssembly (instead of adding these concepts to Core WebAssembly) as was decided way back in CG-05-25. This also means rebasing this repo onto the component-model repo instead of the core
spec
repo (done with a merge commit to avoid losing history).Based on experience since the initial writing, this rewrite of the explainer is also hopefully more concrete, presenting a full AST for adapter modules in Explainer.md (broken into sections that describe each fragment of the AST in sequence) and a full binary decoding of the same AST in Binary.md. Since the proposal no longer adds new things to the existing core spec, the binary format does change significantly (but superficially). Incidentally, work has also started on a proper formal semantics and reference interpreter implementation (in the style of the Core
spec
repo), which will go into thespec
directory of this repo as soon as it gets a bit farther along.(I'll be out for a week after publishing this, so sorry in advance for delayed replies to comments.)