Skip to content

Improve annotation parsing - #2840

Open
zherczeg wants to merge 1 commit into
WebAssembly:mainfrom
zherczeg:annotations
Open

zherczeg wants to merge 1 commit into
WebAssembly:mainfrom
zherczeg:annotations

Conversation

@zherczeg

Copy link
Copy Markdown
Collaborator

No description provided.

@zherczeg

Copy link
Copy Markdown
Collaborator Author

Comment thread src/wast-lexer.cc
ReadChar();
ERROR("unexpected char");
continue;
return TextToken(TokenType::Reserved);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The ; is a reserved character in 3.0
https://webassembly.github.io/spec/core/text/lexical.html#characters

It was not in 2.0, but annotations cannot be checked in lexer.
https://www.w3.org/TR/2025/CRD-wasm-core-2-20250616/#tokens%E2%91%A0

This change should not cause any issues, just yield a different error.

Comment thread src/wast-lexer.cc Outdated
} else if (c == ',' || c == ';' || c == '[' || c == ']' || c == '{' ||
c == '}') {
ReadChar();
return TextToken(TokenType::Reserved);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same story as above.

;;; ERROR: 1
(module
(@a (@))
(@))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Each test contains a valid and an invalid case.

@zherczeg
zherczeg force-pushed the annotations branch 3 times, most recently from 016ab8c to cd9771e Compare August 28, 2026 09:43
Comment thread src/wast-lexer.cc
return GetStringToken(TokenType::LparAnn);
}

while (IsIdChar(PeekChar())) {

@zherczeg zherczeg Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

GetIdChars() accepts ", while we should not.

@zherczeg

Copy link
Copy Markdown
Collaborator Author

Unfortunately this patch adds a lot of minor changes to improve annotation support. Errors remain errors, but their description changes. Error sequences as well, so a stray ; is now an invalid token, and not an ignored unknown character.

To keep the old operation we could add a bool enable_annotations to the lexer. The question is, do we really need to keep the old operation?

I removed the empty file support, since I could not figure out a good way to handle it. The problem is:
(module quote "(@a \09)")
Such spec tests are valid empty modules. However, we got an error.

An option to fix it is adding a bool, which is set to true if annotations are encountered. Then the file is not considered empty. I don't really like it, but no better idea at the moment.

@sbc100 what do you think about these changes?

Comment thread src/wast-lexer.cc
switch (ReadChar()) {
case kEof:
return BareToken(TokenType::Eof);
token_start_ = saved_token_start;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The old operation could be kept with an if (token_type == TokenType::LparAnn) if needed. I don't know if it is needed.

@zherczeg
zherczeg force-pushed the annotations branch 2 times, most recently from 9133417 to d12f13d Compare August 28, 2026 12:36
@zherczeg

Copy link
Copy Markdown
Collaborator Author

@sbc100 Some good news: if the annotation support is fixed, and the gc patches are applied, the whole wasm 3.0 spec test suite is passed.

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.

1 participant