fix trailing IdentifierPart in grammar #641
Conversation
|
- Moved numeric literals to use `notFollowedBy: IdentifierStart` instead of `IdentifierPart`. - Removed `notFollowedBy: IdentifierPart` from string literals, to match the behavior of `solc`.
OmarTawfik
force-pushed
the
fix-trailing-identifier-part-grammar
branch
from
38f76fc to
a6b555c
Compare
| @@ -3833,85 +3845,95 @@ codegen_language_macros::compile!(Language( | |||
| ]) | |||
| ), | |||
| Repeated(name = AsciiStringLiterals, repeated = AsciiStringLiteral), | |||
| Token( | |||
| Enum( | |||
There was a problem hiding this comment.
@OmarTawfik I'm wondering, shouldn't this still be a token? The string literals are considered tokens by solc and it's still defined as scanner in the YAML and DSL v1.
There was a problem hiding this comment.
So, the question is (same for these three), each has a single quote/double quote variation:
HexStringLiteralAsciiStringLiteralUnicodeStringLiteral
Taking the first as an example, in DSL v0/v1, HexStringLiteral was defined as a token, and its definition is Fragment(SingleQuoteHexStringLiteral) || Fragment(DoubleQuoteHexStringLiteral) .. This leads to the CST having a single token named HexStringLiteral but can contain either 0x'FF' or 0x"FF" ..
In DSL v2, I changed that for the string literal to be defined as an Enum, and its definition to SingleQuoteVariant || DoubleQuoteVariant .. Both are stand alone tokens .. Since Enums don’t produce their own NonTerminalKind, we will still end up with one token in the CST, but instead of it being HexStringLiteral, it will be either SingleQuoteHexStringLiteral or DoubleQuoteHexStringLiteral ..
I believe while the change is minor, it makes it much easier to use/deal with, since any operation on the containing string value will have to detect the quote start/end character, but also deal with escape sequences that differ between the two variants based on the quote character. No need to “hide” this piece of information in the tree, only to have to analyze it again afterwards.
Please let me know if you have any concerns on this change.
There was a problem hiding this comment.
Let's reverse that change for now, until after migration is done: #646
notFollowedBy: IdentifierStartinstead ofIdentifierPart.notFollowedBy: IdentifierPartfrom string literals, to match the behavior ofsolc.