Avoid lookahead into interpolation #15518
Conversation
Since an interpolation is really two tokens deep, for the literal part and the interpolated expression, one-char lookahead does not preserve the second token on reset. Make lookahead itself more robust by falling back to a LookaheadScanner for this case. Also add an internal error for the bad reset.
som-snytt
force-pushed
the
issue/15514-interpolation-scan
branch
from
1cd734d to
99707d8
Compare
| @@ -695,10 +695,10 @@ object Scanners { | |||
| getNextToken(token) | |||
| if token == END && !isEndMarker then token = IDENTIFIER | |||
|
|
|||
| def reset() = { | |||
| def reset() = | |||
| if next.token != EMPTY && !isInstanceOf[LookaheadScanner] then report.error(s"Internal: lookAhead/reset would erase next token ${tokenString(next.token)} after ${tokenString(token)}", sourcePos()) | |||
There was a problem hiding this comment.
That should be just as assertion.
There was a problem hiding this comment.
Yes I got carried away because my debug mode was printStackTrace which made it hard to see my special message. Thanks!
There was a problem hiding this comment.
Since it survived the community build, maybe I'll add the assert to the other PR, assuming it must always be an error.
| def lookahead: TokenData = | ||
| if next.token == EMPTY then | ||
| def lookahead: TokenData = | ||
| if next.token != EMPTY then next |
There was a problem hiding this comment.
I think it would be better not to burden lookahead with this logic. Can't be create a Lookahead scanner instead at the call site of lookahead when the assertion is triggered?
There was a problem hiding this comment.
Yes, I went back and forth on where to place the burden. There was no good answer. But the superseding PR, if it works, is conceptually much simpler, as it means lookahead never incurs the advanced token.
There was a problem hiding this comment.
I almost went down the path of, in.canLookAhead, because maybe there are other such states. How could I ever trust lookahead again?
|
Probably superseded by #15519 which just moves the code to postProcessToken. |
Since an interpolation is really two tokens deep, for the literal
part and the interpolated expression, one-char lookahead does not
preserve the second token on reset.
Make lookahead itself more robust by falling back to a LookaheadScanner
for this case.
Also add an internal error for the bad reset.
Fixes #15514