Skip to content

Commit fc759ec

Browse files
committed
Better error recovery in comma-separated lists
Check for trailing commas in parser instead of scanner Instead of passing down a flag, add a bit to regions to say if we are in a comma-separated region. delimited instead of expectedToken
1 parent 1b25f65 commit fc759ec

File tree

10 files changed

+270
-60
lines changed

10 files changed

+270
-60
lines changed

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

Lines changed: 70 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,13 @@ object Parsers {
248248

249249
/** Skip on error to next safe point.
250250
*/
251-
protected def skip(stopAtComma: Boolean): Unit =
251+
protected def skip(): Unit =
252252
val lastRegion = in.currentRegion
253+
val stopAtComma = lastRegion match {
254+
case InParens(_, _, commaSeparated) => commaSeparated
255+
case InBraces(_, commaSeparated) => commaSeparated
256+
case _ => true
257+
}
253258
def atStop =
254259
in.token == EOF
255260
|| ((stopAtComma && in.token == COMMA) || skipStopTokens.contains(in.token)) && (in.currentRegion eq lastRegion)
@@ -277,13 +282,13 @@ object Parsers {
277282
if (in.token == EOF) incompleteInputError(msg)
278283
else
279284
syntaxError(msg, offset)
280-
skip(stopAtComma = true)
285+
skip()
281286

282287
def syntaxErrorOrIncomplete(msg: Message, span: Span): Unit =
283288
if (in.token == EOF) incompleteInputError(msg)
284289
else
285290
syntaxError(msg, span)
286-
skip(stopAtComma = true)
291+
skip()
287292

288293
/** Consume one token of the specified type, or
289294
* signal an error if it is not there.
@@ -351,7 +356,7 @@ object Parsers {
351356
false // it's a statement that might be legal in an outer context
352357
else
353358
in.nextToken() // needed to ensure progress; otherwise we might cycle forever
354-
skip(stopAtComma=false)
359+
skip()
355360
true
356361

357362
in.observeOutdented()
@@ -558,19 +563,55 @@ object Parsers {
558563
def inDefScopeBraces[T](body: => T, rewriteWithColon: Boolean = false): T =
559564
inBracesOrIndented(body, rewriteWithColon)
560565

561-
/** part { `separator` part }
562-
*/
563-
def tokenSeparated[T](separator: Int, part: () => T): List[T] = {
564-
val ts = new ListBuffer[T] += part()
565-
while (in.token == separator) {
566+
/** part { `,` part }
567+
* @param delimited If true, this comma-separated list must surrounded by brackets, parens, or braces.
568+
*/
569+
def commaSeparated[T](part: () => T, delimited: Boolean = true, readFirst: Boolean = true): List[T] = {
570+
val expectedEnd = if (delimited) {
571+
in.currentRegion match {
572+
case InParens(t, outer, _) =>
573+
in.currentRegion = InParens(t, outer, commaSeparated = true)
574+
t match {
575+
case LPAREN => RPAREN
576+
case LBRACKET => RBRACKET
577+
case _ => EMPTY
578+
}
579+
case InBraces(outer, _) =>
580+
in.currentRegion = InBraces(outer, commaSeparated = true)
581+
RBRACE
582+
case _ => EMPTY
583+
}
584+
} else EMPTY
585+
val ts = new ListBuffer[T]
586+
if (readFirst) ts += part()
587+
var done = false
588+
while (in.token == COMMA && !done) {
566589
in.nextToken()
567-
ts += part()
590+
if (in.isAfterLineEnd && (in.token == OUTDENT || (expectedEnd != EMPTY && in.token == expectedEnd))) {
591+
// skip the trailing comma
592+
done = true
593+
} else {
594+
ts += part()
595+
}
596+
}
597+
if (expectedEnd != EMPTY && in.token != expectedEnd) {
598+
// As a side effect, will skip to the nearest safe point, which might be a comma
599+
syntaxErrorOrIncomplete(ExpectedTokenButFound(expectedEnd, in.token))
600+
if (in.token == COMMA) {
601+
ts ++= commaSeparated(part, delimited)
602+
}
603+
}
604+
if (delimited) {
605+
in.currentRegion match {
606+
case InParens(t, outer, true) =>
607+
in.currentRegion = InParens(t, outer, commaSeparated = false)
608+
case InBraces(outer, true) => in.currentRegion = InBraces(outer, commaSeparated = false)
609+
case _ =>
610+
}
568611
}
569612
ts.toList
570613
}
571614

572-
def commaSeparated[T](part: () => T): List[T] = tokenSeparated(COMMA, part)
573-
574615
def inSepRegion[T](f: Region => Region)(op: => T): T =
575616
val cur = in.currentRegion
576617
in.currentRegion = f(cur)
@@ -1386,14 +1427,7 @@ object Parsers {
13861427
else
13871428
Function(params, t)
13881429
}
1389-
def funTypeArgsRest(first: Tree, following: () => Tree) = {
1390-
val buf = new ListBuffer[Tree] += first
1391-
while (in.token == COMMA) {
1392-
in.nextToken()
1393-
buf += following()
1394-
}
1395-
buf.toList
1396-
}
1430+
13971431
var isValParamList = false
13981432

13991433
val t =
@@ -1409,11 +1443,10 @@ object Parsers {
14091443
val ts = funArgType() match {
14101444
case Ident(name) if name != tpnme.WILDCARD && in.isColon() =>
14111445
isValParamList = true
1412-
funTypeArgsRest(
1413-
typedFunParam(paramStart, name.toTermName, imods),
1414-
() => typedFunParam(in.offset, ident(), imods))
1446+
typedFunParam(paramStart, name.toTermName, imods) :: commaSeparated(
1447+
() => typedFunParam(in.offset, ident(), imods), readFirst = false)
14151448
case t =>
1416-
funTypeArgsRest(t, funArgType)
1449+
t :: commaSeparated(funArgType,readFirst = false)
14171450
}
14181451
accept(RPAREN)
14191452
if isValParamList || in.isArrow then
@@ -2538,7 +2571,7 @@ object Parsers {
25382571
if (leading == LBRACE || in.token == CASE)
25392572
enumerators()
25402573
else {
2541-
val pats = patternsOpt()
2574+
val pats = patternsOpt(delimited=false)
25422575
val pat =
25432576
if (in.token == RPAREN || pats.length > 1) {
25442577
wrappedEnums = false
@@ -2730,7 +2763,7 @@ object Parsers {
27302763
case USCORE =>
27312764
wildcardIdent()
27322765
case LPAREN =>
2733-
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
2766+
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(delimited=true))) }
27342767
case QUOTE =>
27352768
simpleExpr(Location.InPattern)
27362769
case XMLSTART =>
@@ -2766,11 +2799,11 @@ object Parsers {
27662799

27672800
/** Patterns ::= Pattern [`,' Pattern]
27682801
*/
2769-
def patterns(location: Location = Location.InPattern): List[Tree] =
2770-
commaSeparated(() => pattern(location))
2802+
def patterns(delimited: Boolean, location: Location = Location.InPattern): List[Tree] =
2803+
commaSeparated(() => pattern(location), delimited)
27712804

2772-
def patternsOpt(location: Location = Location.InPattern): List[Tree] =
2773-
if (in.token == RPAREN) Nil else patterns(location)
2805+
def patternsOpt(location: Location = Location.InPattern, delimited: Boolean = true): List[Tree] =
2806+
if (in.token == RPAREN) Nil else patterns(delimited, location)
27742807

27752808
/** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’
27762809
* | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
@@ -3119,7 +3152,7 @@ object Parsers {
31193152
*/
31203153
def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = {
31213154
val offset = accept(leading)
3122-
commaSeparated(importExpr(mkTree)) match {
3155+
commaSeparated(importExpr(mkTree), delimited=false) match {
31233156
case t :: rest =>
31243157
// The first import should start at the start offset of the keyword.
31253158
val firstPos =
@@ -3196,9 +3229,9 @@ object Parsers {
31963229
}
31973230
else ImportSelector(from)
31983231

3199-
def importSelectors(idOK: Boolean): List[ImportSelector] =
3232+
def importSelector(idOK: Boolean)(): ImportSelector =
32003233
val isWildcard = in.token == USCORE || in.token == GIVEN || isIdent(nme.raw.STAR)
3201-
val selector = atSpan(in.offset) {
3234+
atSpan(in.offset) {
32023235
in.token match
32033236
case USCORE => wildcardSelector()
32043237
case GIVEN => givenSelector()
@@ -3208,13 +3241,6 @@ object Parsers {
32083241
if !idOK then syntaxError(i"named imports cannot follow wildcard imports")
32093242
namedSelector(termIdent())
32103243
}
3211-
val rest =
3212-
if in.token == COMMA then
3213-
in.nextToken()
3214-
importSelectors(idOK = idOK && !isWildcard)
3215-
else
3216-
Nil
3217-
selector :: rest
32183244

32193245
def importSelection(qual: Tree): Tree =
32203246
if in.isIdent(nme.as) && qual.isInstanceOf[RefTree] then
@@ -3232,7 +3258,7 @@ object Parsers {
32323258
case GIVEN =>
32333259
mkTree(qual, givenSelector() :: Nil)
32343260
case LBRACE =>
3235-
mkTree(qual, inBraces(importSelectors(idOK = true)))
3261+
mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true))))
32363262
case _ =>
32373263
if isIdent(nme.raw.STAR) then
32383264
mkTree(qual, wildcardSelector() :: Nil)
@@ -3289,7 +3315,7 @@ object Parsers {
32893315
var lhs = first match {
32903316
case id: Ident if in.token == COMMA =>
32913317
in.nextToken()
3292-
id :: commaSeparated(() => termIdent())
3318+
id :: commaSeparated(() => termIdent(), delimited=false)
32933319
case _ =>
32943320
first :: Nil
32953321
}
@@ -3560,7 +3586,7 @@ object Parsers {
35603586
val id = termIdent()
35613587
if (in.token == COMMA) {
35623588
in.nextToken()
3563-
val ids = commaSeparated(() => termIdent())
3589+
val ids = commaSeparated(() => termIdent(), delimited=false)
35643590
PatDef(mods1, id :: ids, TypeTree(), EmptyTree)
35653591
}
35663592
else {
@@ -3764,7 +3790,7 @@ object Parsers {
37643790
val derived =
37653791
if (isIdent(nme.derives)) {
37663792
in.nextToken()
3767-
tokenSeparated(COMMA, () => convertToTypeId(qualId()))
3793+
commaSeparated(() => convertToTypeId(qualId()), delimited=false)
37683794
}
37693795
else Nil
37703796
possibleTemplateStart()

compiler/src/dotty/tools/dotc/parsing/Scanners.scala

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ object Scanners {
276276
/** Are we in a `${ }` block? such that RBRACE exits back into multiline string. */
277277
private def inMultiLineInterpolatedExpression =
278278
currentRegion match {
279-
case InBraces(InString(true, _)) => true
279+
case InBraces(InString(true, _), _) => true
280280
case _ => false
281281
}
282282

@@ -310,7 +310,7 @@ object Scanners {
310310
dropBraces()
311311
case RPAREN | RBRACKET =>
312312
currentRegion match {
313-
case InParens(prefix, outer) if prefix + 1 == lastToken => currentRegion = outer
313+
case InParens(prefix, outer, _) if prefix + 1 == lastToken => currentRegion = outer
314314
case _ =>
315315
}
316316
case STRINGLIT =>
@@ -645,13 +645,6 @@ object Scanners {
645645
insert(OUTDENT, offset)
646646
currentRegion = r.outer
647647
case _ =>
648-
lookAhead()
649-
if isAfterLineEnd
650-
&& (token == RPAREN || token == RBRACKET || token == RBRACE || token == OUTDENT)
651-
then
652-
() /* skip the trailing comma */
653-
else
654-
reset()
655648
case END =>
656649
if !isEndMarker then token = IDENTIFIER
657650
case COLON =>
@@ -1441,7 +1434,7 @@ object Scanners {
14411434
def proposeKnownWidth(width: IndentWidth, lastToken: Token) =
14421435
if knownWidth == null then
14431436
this match
1444-
case InParens(_, _) if lastToken != LPAREN =>
1437+
case InParens(_, _, _) if lastToken != LPAREN =>
14451438
useOuterWidth()
14461439
case _ =>
14471440
knownWidth = width
@@ -1452,8 +1445,8 @@ object Scanners {
14521445

14531446
private def delimiter = this match
14541447
case _: InString => "}(in string)"
1455-
case InParens(LPAREN, _) => ")"
1456-
case InParens(LBRACKET, _) => "]"
1448+
case InParens(LPAREN, _, _) => ")"
1449+
case InParens(LBRACKET, _, _) => "]"
14571450
case _: InBraces => "}"
14581451
case _: InCase => "=>"
14591452
case _: Indented => "UNDENT"
@@ -1468,8 +1461,8 @@ object Scanners {
14681461
end Region
14691462

14701463
case class InString(multiLine: Boolean, outer: Region) extends Region
1471-
case class InParens(prefix: Token, outer: Region) extends Region
1472-
case class InBraces(outer: Region) extends Region
1464+
case class InParens(prefix: Token, outer: Region, commaSeparated: Boolean = false) extends Region
1465+
case class InBraces(outer: Region, commaSeparated: Boolean = false) extends Region
14731466
case class InCase(outer: Region) extends Region
14741467

14751468
/** A class describing an indentation region.

compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ object MarkupParsers {
417417

418418
/** xScalaPatterns ::= patterns
419419
*/
420-
def xScalaPatterns: List[Tree] = escapeToScala(parser.patterns(), "pattern")
420+
def xScalaPatterns: List[Tree] = escapeToScala(parser.patterns(delimited = false), "pattern")
421421

422422
def reportSyntaxError(offset: Int, str: String): Unit = parser.syntaxError(str, offset)
423423
def reportSyntaxError(str: String): Unit = {
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:21 ----------------------------------------------------
2+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
3+
| ^
4+
| ')' expected, but integer literal found
5+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:26 ----------------------------------------------------
6+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
7+
| ^^^
8+
| ':' expected, but identifier found
9+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:42 ----------------------------------------------------
10+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
11+
| ^
12+
| ')' expected, but integer literal found
13+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:47 ----------------------------------------------------
14+
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
15+
| ^
16+
| ':' expected, but '=' found
17+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:11:16 ---------------------------------------------------
18+
11 | case Plus(4 1) => // error
19+
| ^
20+
| ')' expected, but integer literal found
21+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:16 ---------------------------------------------------
22+
12 | case Plus(4 5 6 7, 1, 2 3) => // error // error
23+
| ^
24+
| ')' expected, but integer literal found
25+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:28 ---------------------------------------------------
26+
12 | case Plus(4 5 6 7, 1, 2 3) => // error // error
27+
| ^
28+
| ')' expected, but integer literal found
29+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:12 ---------------------------------------------------
30+
14 | val x: A[T=Int, T=Int] = ??? // error // error
31+
| ^
32+
| ']' expected, but '=' found
33+
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:19 ---------------------------------------------------
34+
14 | val x: A[T=Int, T=Int] = ??? // error // error
35+
| ^
36+
| ']' expected, but '=' found
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
class A[T]
2+
object o {
3+
def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
4+
5+
case class Plus(a: Int, b: Int)
6+
7+
object Plus {
8+
def unapply(r: Int): Plus = Plus(r - 1, 1)
9+
}
10+
5 match {
11+
case Plus(4 1) => // error
12+
case Plus(4 5 6 7, 1, 2 3) => // error // error
13+
}
14+
val x: A[T=Int, T=Int] = ??? // error // error
15+
}

tests/neg/i1679.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
class A[T]
22
object o {
33
// Testing compiler crash, this test should be modified when named type argument are completely implemented
4-
val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error
4+
val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error: ']' expected, but '=' found
55
}

tests/neg/t11900.check

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-- Error: tests/neg/t11900.scala:44:16 ---------------------------------------------------------------------------------
2+
44 | a => a + 1, // error: weird comma
3+
| ^
4+
| end of statement expected but ',' found
5+
-- Error: tests/neg/t11900.scala:48:16 ---------------------------------------------------------------------------------
6+
48 | println("a"), // error: weird comma
7+
| ^
8+
| end of statement expected but ',' found
9+
-- Error: tests/neg/t11900.scala:52:16 ---------------------------------------------------------------------------------
10+
52 | println("b"), // error: weird comma
11+
| ^
12+
| end of statement expected but ',' found
13+
-- [E032] Syntax Error: tests/neg/t11900.scala:64:8 --------------------------------------------------------------------
14+
64 | _*, // error
15+
| ^
16+
| pattern expected
17+
|
18+
| longer explanation available when compiling with `-explain`

0 commit comments

Comments
 (0)