[Parser] Don't modify the current token kind when cutting off parsing #40065
Conversation
|
@swift-ci Please smoke test |
There was a problem hiding this comment.
I assume this was from me, obviously doesn't matter but could remove if you wanted it to be a little neater :P
There was a problem hiding this comment.
Whatever. The file hit the issue before and now doesn’t hit it any longer so I don’t want to change it anymore.
ahoppen
force-pushed
the
pr/lexing-cutoff
branch
from
37cc359 to
55f1bf9
Compare
|
@swift-ci Please smoke test |
ahoppen
force-pushed
the
pr/lexing-cutoff
branch
from
55f1bf9 to
21aa2d3
Compare
|
@swift-ci Please smoke test |
ahoppen
force-pushed
the
pr/lexing-cutoff
branch
from
21aa2d3 to
d725c51
Compare
|
@swift-ci Please smoke test |
lib/Parse/Parser.cpp
Outdated
There was a problem hiding this comment.
Can this ever be a nullptr? BufferID is set from BaseLexer->getBufferID() which would suggest not, in which case maybe &BaseLexer makes more sense?
There was a problem hiding this comment.
Good idea. Changed it to a reference.
There was a problem hiding this comment.
The crash here was the assertion failure right? Would it be worth adding // REQUIRES: asserts or a comment to that effect in here? The test does work either way so maybe doesn't matter, but IMO it'd be nice to differentiate it from the existing structure_overflow*.swift ones.
Previously, when we reached the maximum nesting level, we changed the current token’s kind to an EOF token. A lot of places in the parser are not set up to expect this token change. The intended workaround was to check whether pushing a structure marker failed (which would change the token kind) and bail out parsing if this happened. This was fragile and caused assertion failures in assert builds. Instead of changing the current token’s kind, and failing to push the structure marker, let the lexer know that it should cut off lexing, essentially making the input buffer stop at the current position. The parser will continue to consume its current token (`Parser.Tok`) and the next token that’s already lexed in the lexer (`Lexer.NextToken`) before reaching the emulated EOF token. Thus two more tokens are parsed than before, but that shouldn’t make much of a difference.
ahoppen
force-pushed
the
pr/lexing-cutoff
branch
from
d725c51 to
b888dc0
Compare
|
@swift-ci Please smoke test |
2 participants
Instead, let the lexer know that it should interpret the next token to be lexed as an EOF token. This makes parsing cutoff a lot more stable because the parser is not expecting the current token kind to change in a lot of places.
Technically, this causes two more tokens to be parsed than in the previous implementation (namely
Parser.TokandLexer.NextToken) but since we bail out at a fairly low nesting level, this shouldn't make a big difference.