Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Parser] Parse annotations, including source map comments #6345

Merged
merged 2 commits into from
Feb 27, 2024

Conversation

tlively
Copy link
Member

@tlively tlively commented Feb 24, 2024

Parse annotations using the standards-track (@annotation ...) format as well
as the ;;@ source-map:0:1 format. Have the lexer implicitly collect
annotations while it skips whitespace and add lexer APIs to access the
annotations since the last token was parsed. Collect annotations before parsing
each instruction and pass the annotations explicitly to the parser and parser
context functions for instructions. Add an API to IRBuilder to set a debug
location to be attached to the next visited or created instruction and use it
from the parser.

Parse annotations using the standards-track `(@annotation ...)` format as well
as the `;;@ source-map:0:1` format. Have the lexer implicitly collect
annotations while it skips whitespace and add lexer APIs to access the
annotations since the last token was parsed. Collect annotations before parsing
each instruction and pass the annotations explicitly to the parser and parser
context functions for instructions. Add an API to `IRBuilder` to set a debug
location to be attached to the next visited or created instruction and use it
from the parser.
@tlively tlively requested a review from kripken February 24, 2024 00:58
@tlively
Copy link
Member Author

tlively commented Feb 24, 2024

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@@ -649,11 +668,14 @@ Result<> IRBuilder::visitTryStart(Try* tryy, Name label) {
}

Result<> IRBuilder::visitTryTableStart(TryTable* trytable, Name label) {
applyDebugLoc(trytable);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we applyDebugLoc in control flow structures in this file but not to e.g. visitStructSet?

Copy link
Member Author

Choose a reason for hiding this comment

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

For normal instructions, the applyDebugLoc in push is sufficient. For control flow, however, we need to apply the debug location at the beginning of the scope. Since the control flow isn't pushed until the end of the scope, we can't rely on the same applyDebugLoc as for normal instructions.

Copy link
Member

@kripken kripken Feb 27, 2024

Choose a reason for hiding this comment

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

Ah, yes, thanks, that makes sense, and I think we have similar code for the old parser in fact...

;; Annotations
(@annotation this is a meaningless (@annotation ) ;; This is still a comment ))
it spans multiple lines just fine and can include $ids 0x42 numbers and "strings"
)
Copy link
Member

Choose a reason for hiding this comment

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

Should this not have a CHECK?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, the parser never does anything with it, so it's essentially a comment.

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't it need to roundtrip it?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. There is a specific kind of annotation ((@custom ...)) that specifies arbitrary custom section contents that should be round-tripped, but we don't support that yet.

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks, that's what I was missing.

@tlively tlively merged commit f8b07f7 into main Feb 27, 2024
28 checks passed
@tlively tlively deleted the parser-annotations branch February 27, 2024 01:55
radekdoulik pushed a commit to dotnet/binaryen that referenced this pull request Jul 12, 2024
…y#6345)

Parse annotations using the standards-track `(@annotation ...)` format as well
as the `;;@ source-map:0:1` format. Have the lexer implicitly collect
annotations while it skips whitespace and add lexer APIs to access the
annotations since the last token was parsed. Collect annotations before parsing
each instruction and pass the annotations explicitly to the parser and parser
context functions for instructions. Add an API to `IRBuilder` to set a debug
location to be attached to the next visited or created instruction and use it
from the parser.
@gkdn gkdn mentioned this pull request Aug 31, 2024
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