Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Text format: new abbreviation for inline data? #29

Closed
Tracked by #43
tlively opened this issue Oct 19, 2022 · 6 comments
Closed
Tracked by #43

Text format: new abbreviation for inline data? #29

tlively opened this issue Oct 19, 2022 · 6 comments

Comments

@tlively
Copy link
Member

tlively commented Oct 19, 2022

The current spec gives this abbreviation for specifying data inline in a memory definition:

'(' 'memory' id? '(' 'data' b*:datastring ')' ')'

The memory limits are inferred from the size of the data string. For back compat, this abbreviation needs to continue specifying a 32-bit memory, but should we add a similar abbreviation for 64-bit memories? As an example:

'(' 'memory' id? '(' 'data' 'i64' b*:datastring ')' ')'

We could also choose not to provide such an abbreviation.

@rossberg
Copy link
Member

Yes, makes a lot of sense to me. However, I think the syntax ought to be

'(' 'memory' id? idxtype '(' 'data' datastring ')' ')'

Reason: The index type is part of the memory type, not the data. Inline data (or inline elem) only modifies the preceding syntax of memory (or table) definitions in so far that their limits are omitted.

@tlively
Copy link
Member Author

tlively commented Oct 21, 2022

I agree that syntax would make more sense, but it would mean that a parser that doesn't expand abbreviations wouldn't know from looking at the idxtype token whether to parse a memtype or an idxtype followed by data. That's not a huge problem, but it is a little annoying.

@rossberg
Copy link
Member

@tlively, I guess I'm willing to live with that small annoyance. :) The analogous shorthand for tables already works that way, although the order is different there, so the annoyance doesn't come up, I suppose.

@WebAssembly WebAssembly deleted a comment from Zackaryjacob Nov 21, 2022
@WebAssembly WebAssembly deleted a comment from Zackaryjacob Nov 21, 2022
@WebAssembly WebAssembly deleted a comment from Zackaryjacob Nov 21, 2022
sbc100 pushed a commit that referenced this issue Jun 12, 2024
Switch order of load/store immediates in binary format
@bvisness
Copy link
Collaborator

bvisness commented Aug 13, 2024

The current version of the spec already supports the syntax from this comment, and at least wasm-tools has implemented it. Also, I recall from a discussion with @tlively in Pittsburgh that naive expansion of abbreviations in the text format was already not viable because of existing things in the spec.

Is there anything else to do on this issue or could it be closed?

@tlively
Copy link
Member Author

tlively commented Aug 13, 2024

I'm happy to close it without adding an abbreviation if everyone else is happy to close it. @sbc100?

@sbc100
Copy link
Member

sbc100 commented Aug 13, 2024

Yes, I believe the syntax from #29 (comment) is already supported and tested in test/core/float_memory64.wast and test/core/memory64.wast.

@sbc100 sbc100 closed this as completed Aug 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants