Skip to content

Adjust canonical ABI option syntax to be less verbose#42

Merged
lukewagner merged 1 commit intoWebAssembly:mainfrom
alexcrichton:less-verbose
Jun 9, 2022
Merged

Adjust canonical ABI option syntax to be less verbose#42
lukewagner merged 1 commit intoWebAssembly:mainfrom
alexcrichton:less-verbose

Conversation

@alexcrichton
Copy link
Collaborator

Currently the syntax for specifying a memory with a canonical option is:

(memory (core memory $memory))

and optionally instance alias sugar can also be used:

(memory (core memory $libc "memory"))

This PR proposes changing these two syntaxes to:

(memory $memory)
(memory $libc "memory")

with the theory that the "core" part is already implied by the canonical
option itself and otherwise saying "memory" twice is redundant.

@lukewagner
Copy link
Member

Totally agreed the current syntax feels overly verbose in these scenarios so thanks for bringing this up!

So, first, I was already on the edge about whether we needed the core-prefix here (given the static context of "this is definitely a core index"), so agreed we should drop that part.

If we wanted to drop the (sort ...) that wraps the <core:instanceidx> <name>, then I think we'd need to define some custom syntactic sugar just for the instantiate instruction because, as is, if the AST has, e.g., (realloc <core:funcidx>), then the inline alias sugar only lets you substitute in a (func $inst "exp") for the <core:funcidx>, producing (realloc (func $inst "exp")). memory is a more ambiguous case since memory is a case of sort, but in this context, the memory token is serving as a label of an immediate, which could have just as well been named not-memory, e.g. into. Because the sugar-substitution is purely lexical (it doesn't care about sort vs. immediate), you could say that the inline alias sugar already allows you to write what you're proposing in this PR. (FWIW, I tried to think of a better immediate label than memory to this wasn't a special case, but I couldn't find one: into only makes sense for lifting; mem feels contrived.)

So: drop the core-prefix and but keep the (func ...) for realloc/post-return?

@alexcrichton
Copy link
Collaborator Author

👍 sounds reasonable to me!

Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

Thanks! Could you also update examples/SharedEverythingDynamicLinking.md (where, it appears, I wasn't even already fully following the explainer...).

Comment on lines +620 to +621
| (realloc (func <core:funcidx>))
| (post-return (func <core:funcidx>))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| (realloc (func <core:funcidx>))
| (post-return (func <core:funcidx>))
| (realloc <core:funcidx>)
| (post-return <core:funcidx>)

The idea being that you can either write (canon lift $f (realloc $r) (func $l)) or, via inline-alias-sugar, (canon lift $f (realloc (func $i "r")) (func $l)).

Currently the syntax for specifying a memory with a canonical option is:

    (memory (core memory $memory))

and optionally instance alias sugar can also be used:

    (memory (core memory $libc "memory"))

This PR proposes changing these two syntaxes to:

    (memory $memory)
    (memory $libc "memory")

with the theory that the "core" part is already implied by the canonical
option itself and otherwise saying "memory" twice is redundant.
@alexcrichton
Copy link
Collaborator Author

Sure thing, should be updated now I believe.

@lukewagner
Copy link
Member

Thanks!

@lukewagner lukewagner merged commit 5a3d256 into WebAssembly:main Jun 9, 2022
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.

2 participants