-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Parse] Lexer build backtick trivia around espaced identifier token #13631
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change itself looks good 👍
Comments for test cases.
@@ -0,0 +1,4 @@ | |||
// RUN: %swift-syntax-test -input-source-filename %s -serialize-raw-tree > %t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are actively developing/modifying syntax tree structure, we expect this output will be obsoleted very soon. I don't think we need this test.
see also: #13485 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks review.
Ah I have also felt that this test is fragile .
Ok, I will remove this.
@@ -0,0 +1,4 @@ | |||
// RUN: %swift-syntax-test -input-source-filename %s -dump-full-tokens > %t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially, -dump-full-tokens
is for debug use only. We don't want to keep this output as a formal test case.
Alternatively, could you change this to FileCheck
based test?
Like:
// CHECK-LABEL: 3:25
// CHECK-NEXT: (token identifier
// CHECK-NEXT: (trivia block_comment
// CHECK-NEXT: (trivia space 1)
// CHECK-NEXT: (trivia backtick 1)
// CHECK-NEXT: (text="if")
// CHECK-NEXT: (trivia backtick 1)
// CHECK-LABEL: 4:27
// CHECK-NEXT: (token identifier
// CHECK-NEXT: ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will try it.
e1ac59f
to
bc88330
Compare
I updated commits. Please check it. |
@swift-ci Please test |
@swift-ci Please test |
thank you |
There was a logic that build backtick trivia before and after of escaped identifier token at two places.
First is at
swift::tokenizeWithTrivia
inParser.cpp
.Second is at
SyntaxParsingContext::addToken
inSyntaxParsingContext.cpp
.This PR moves this logic into Lexer and makes them common.
First commit is adding of testcase for existing.
To check first case, it runs
swift-syntax-test -dump-full-tokens
.To check second case, it runs
swift-syntax-test -serialize-raw-tree
.Second commit is actual patch.