Skip to content

Avoid lookahead into interpolation #15518

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Avoid lookahead into interpolation #15518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Copy link
Contributor

Choose a reason for hiding this comment

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

That should be just as assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I got carried away because my debug mode was printStackTrace which made it hard to see my special message. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it survived the community build, maybe I'll add the assert to the other PR, assuming it must always be an error.

next.copyFrom(this)
this.copyFrom(prev)
}

def closeIndented() = currentRegion match
case r: Indented if !r.isOutermost => insert(OUTDENT, offset)
Expand Down Expand Up @@ -1080,16 +1080,24 @@ object Scanners {
// Lookahead ---------------------------------------------------------------

/** The next token after this one.
*
* The token is computed via fetchToken, so complex two word
* tokens such as CASECLASS are not recognized.
* Newlines and indent/unindent tokens are skipped.
*
* Since interpolation prefetches both STRINGPART and an expression,
* use a scanner for rare case. Otherwise the next is overwritten on reset.
*/
def lookahead: TokenData =
if next.token == EMPTY then
def lookahead: TokenData =
if next.token != EMPTY then next
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I almost went down the path of, in.canLookAhead, because maybe there are other such states. How could I ever trust lookahead again?

else if token == INTERPOLATIONID then
val scan = LookaheadScanner()
scan.nextToken()
scan
else
lookAhead()
reset()
next
next

class LookaheadScanner(val allowIndent: Boolean = false) extends Scanner(source, offset) {
override def languageImportContext = Scanner.this.languageImportContext
Expand Down
4 changes: 4 additions & 0 deletions tests/pos/i15514.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

object Main { s"Hello $Main.toStr!" }

object Alt { s"Hello ${Alt}.toStr!" }