Reduce repetition in the text format of canonical options#628
Merged
alexcrichton merged 1 commit intobytecodealliance:mainfrom Jun 9, 2022
Merged
Reduce repetition in the text format of canonical options#628alexcrichton merged 1 commit intobytecodealliance:mainfrom
alexcrichton merged 1 commit intobytecodealliance:mainfrom
Conversation
Member
Author
|
I'm curious what you think of this @peterhuene and if you're ok I can try to send appropriate PRs to the upstream spec as well. |
Member
|
I'm all for it. I agree it is needlessly verbose and there's precedent in other parts of the grammar for this syntax, e.g. instantiation of modules and components are done by index too. |
peterhuene
approved these changes
Jun 8, 2022
Member
peterhuene
left a comment
peterhuene
left a comment
There was a problem hiding this comment.
Love it; makes the canonical options easier to use.
Good to merge once Luke approves the spec change.
alexcrichton
force-pushed
the
less-verbose
branch
from
bbd043f to
3cb1b01
Compare
This was referenced Jun 8, 2022
alexcrichton
force-pushed
the
less-verbose
branch
from
3cb1b01 to
f152be0
Compare
Member
Author
|
Adjusted for the syntax settled on WebAssembly/component-model#42 |
Currently canonical options have a textual syntax that looks like:
(memory (core memory $name))
but personally this seems a bit wordy and repetitive to me. The "core"
part is already implied by the canonical lifting and lowering, and
otherwise the "memory" part is repeated. Alternatively I think it might
be a bit easier to read/write syntax of the form:
(memory $name)
(realloc $realloc)
without any loss in clarity. Additionally though this would still keep
the ability to reference and instance along the lines of:
(memory $libc "memory")
alexcrichton
force-pushed
the
less-verbose
branch
from
f152be0 to
1f594ca
Compare
code-terror
pushed a commit
to code-terror/wasm-tools
that referenced
this pull request
Aug 24, 2022
…liance#628) Currently canonical options have a textual syntax that looks like: (memory (core memory $name)) but personally this seems a bit wordy and repetitive to me. The "core" part is already implied by the canonical lifting and lowering, and otherwise the "memory" part is repeated. Alternatively I think it might be a bit easier to read/write syntax of the form: (memory $name) (realloc $realloc) without any loss in clarity. Additionally though this would still keep the ability to reference and instance along the lines of: (memory $libc "memory")
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Currently canonical options have a textual syntax that looks like:
but personally this seems a bit wordy and repetitive to me. The "core"
part is already implied by the canonical lifting and lowering, and
otherwise the "memory" part is repeated. Alternatively I think it might
be a bit easier to read/write syntax of the form:
without any loss in clarity. Additionally though this would still keep
the ability to reference and instance along the lines of: