Skip to content

Commit

Permalink
Better error recovery in comma-separated lists
Browse files Browse the repository at this point in the history
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
  • Loading branch information
adampauls committed Mar 4, 2022
1 parent 1b25f65 commit fc759ec
Show file tree
Hide file tree
Showing 10 changed files with 270 additions and 60 deletions.
114 changes: 70 additions & 44 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,13 @@ object Parsers {

/** Skip on error to next safe point.
*/
protected def skip(stopAtComma: Boolean): Unit =
protected def skip(): Unit =
val lastRegion = in.currentRegion
val stopAtComma = lastRegion match {
case InParens(_, _, commaSeparated) => commaSeparated
case InBraces(_, commaSeparated) => commaSeparated
case _ => true
}
def atStop =
in.token == EOF
|| ((stopAtComma && in.token == COMMA) || skipStopTokens.contains(in.token)) && (in.currentRegion eq lastRegion)
Expand Down Expand Up @@ -277,13 +282,13 @@ object Parsers {
if (in.token == EOF) incompleteInputError(msg)
else
syntaxError(msg, offset)
skip(stopAtComma = true)
skip()

def syntaxErrorOrIncomplete(msg: Message, span: Span): Unit =
if (in.token == EOF) incompleteInputError(msg)
else
syntaxError(msg, span)
skip(stopAtComma = true)
skip()

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

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

/** part { `separator` part }
*/
def tokenSeparated[T](separator: Int, part: () => T): List[T] = {
val ts = new ListBuffer[T] += part()
while (in.token == separator) {
/** part { `,` part }
* @param delimited If true, this comma-separated list must surrounded by brackets, parens, or braces.
*/
def commaSeparated[T](part: () => T, delimited: Boolean = true, readFirst: Boolean = true): List[T] = {
val expectedEnd = if (delimited) {
in.currentRegion match {
case InParens(t, outer, _) =>
in.currentRegion = InParens(t, outer, commaSeparated = true)
t match {
case LPAREN => RPAREN
case LBRACKET => RBRACKET
case _ => EMPTY
}
case InBraces(outer, _) =>
in.currentRegion = InBraces(outer, commaSeparated = true)
RBRACE
case _ => EMPTY
}
} else EMPTY
val ts = new ListBuffer[T]
if (readFirst) ts += part()
var done = false
while (in.token == COMMA && !done) {
in.nextToken()
ts += part()
if (in.isAfterLineEnd && (in.token == OUTDENT || (expectedEnd != EMPTY && in.token == expectedEnd))) {
// skip the trailing comma
done = true
} else {
ts += part()
}
}
if (expectedEnd != EMPTY && in.token != expectedEnd) {
// As a side effect, will skip to the nearest safe point, which might be a comma
syntaxErrorOrIncomplete(ExpectedTokenButFound(expectedEnd, in.token))
if (in.token == COMMA) {
ts ++= commaSeparated(part, delimited)
}
}
if (delimited) {
in.currentRegion match {
case InParens(t, outer, true) =>
in.currentRegion = InParens(t, outer, commaSeparated = false)
case InBraces(outer, true) => in.currentRegion = InBraces(outer, commaSeparated = false)
case _ =>
}
}
ts.toList
}

def commaSeparated[T](part: () => T): List[T] = tokenSeparated(COMMA, part)

def inSepRegion[T](f: Region => Region)(op: => T): T =
val cur = in.currentRegion
in.currentRegion = f(cur)
Expand Down Expand Up @@ -1386,14 +1427,7 @@ object Parsers {
else
Function(params, t)
}
def funTypeArgsRest(first: Tree, following: () => Tree) = {
val buf = new ListBuffer[Tree] += first
while (in.token == COMMA) {
in.nextToken()
buf += following()
}
buf.toList
}

var isValParamList = false

val t =
Expand All @@ -1409,11 +1443,10 @@ object Parsers {
val ts = funArgType() match {
case Ident(name) if name != tpnme.WILDCARD && in.isColon() =>
isValParamList = true
funTypeArgsRest(
typedFunParam(paramStart, name.toTermName, imods),
() => typedFunParam(in.offset, ident(), imods))
typedFunParam(paramStart, name.toTermName, imods) :: commaSeparated(
() => typedFunParam(in.offset, ident(), imods), readFirst = false)
case t =>
funTypeArgsRest(t, funArgType)
t :: commaSeparated(funArgType,readFirst = false)
}
accept(RPAREN)
if isValParamList || in.isArrow then
Expand Down Expand Up @@ -2538,7 +2571,7 @@ object Parsers {
if (leading == LBRACE || in.token == CASE)
enumerators()
else {
val pats = patternsOpt()
val pats = patternsOpt(delimited=false)
val pat =
if (in.token == RPAREN || pats.length > 1) {
wrappedEnums = false
Expand Down Expand Up @@ -2730,7 +2763,7 @@ object Parsers {
case USCORE =>
wildcardIdent()
case LPAREN =>
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(delimited=true))) }
case QUOTE =>
simpleExpr(Location.InPattern)
case XMLSTART =>
Expand Down Expand Up @@ -2766,11 +2799,11 @@ object Parsers {

/** Patterns ::= Pattern [`,' Pattern]
*/
def patterns(location: Location = Location.InPattern): List[Tree] =
commaSeparated(() => pattern(location))
def patterns(delimited: Boolean, location: Location = Location.InPattern): List[Tree] =
commaSeparated(() => pattern(location), delimited)

def patternsOpt(location: Location = Location.InPattern): List[Tree] =
if (in.token == RPAREN) Nil else patterns(location)
def patternsOpt(location: Location = Location.InPattern, delimited: Boolean = true): List[Tree] =
if (in.token == RPAREN) Nil else patterns(delimited, location)

/** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’
* | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
Expand Down Expand Up @@ -3119,7 +3152,7 @@ object Parsers {
*/
def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = {
val offset = accept(leading)
commaSeparated(importExpr(mkTree)) match {
commaSeparated(importExpr(mkTree), delimited=false) match {
case t :: rest =>
// The first import should start at the start offset of the keyword.
val firstPos =
Expand Down Expand Up @@ -3196,9 +3229,9 @@ object Parsers {
}
else ImportSelector(from)

def importSelectors(idOK: Boolean): List[ImportSelector] =
def importSelector(idOK: Boolean)(): ImportSelector =
val isWildcard = in.token == USCORE || in.token == GIVEN || isIdent(nme.raw.STAR)
val selector = atSpan(in.offset) {
atSpan(in.offset) {
in.token match
case USCORE => wildcardSelector()
case GIVEN => givenSelector()
Expand All @@ -3208,13 +3241,6 @@ object Parsers {
if !idOK then syntaxError(i"named imports cannot follow wildcard imports")
namedSelector(termIdent())
}
val rest =
if in.token == COMMA then
in.nextToken()
importSelectors(idOK = idOK && !isWildcard)
else
Nil
selector :: rest

def importSelection(qual: Tree): Tree =
if in.isIdent(nme.as) && qual.isInstanceOf[RefTree] then
Expand All @@ -3232,7 +3258,7 @@ object Parsers {
case GIVEN =>
mkTree(qual, givenSelector() :: Nil)
case LBRACE =>
mkTree(qual, inBraces(importSelectors(idOK = true)))
mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true))))
case _ =>
if isIdent(nme.raw.STAR) then
mkTree(qual, wildcardSelector() :: Nil)
Expand Down Expand Up @@ -3289,7 +3315,7 @@ object Parsers {
var lhs = first match {
case id: Ident if in.token == COMMA =>
in.nextToken()
id :: commaSeparated(() => termIdent())
id :: commaSeparated(() => termIdent(), delimited=false)
case _ =>
first :: Nil
}
Expand Down Expand Up @@ -3560,7 +3586,7 @@ object Parsers {
val id = termIdent()
if (in.token == COMMA) {
in.nextToken()
val ids = commaSeparated(() => termIdent())
val ids = commaSeparated(() => termIdent(), delimited=false)
PatDef(mods1, id :: ids, TypeTree(), EmptyTree)
}
else {
Expand Down Expand Up @@ -3764,7 +3790,7 @@ object Parsers {
val derived =
if (isIdent(nme.derives)) {
in.nextToken()
tokenSeparated(COMMA, () => convertToTypeId(qualId()))
commaSeparated(() => convertToTypeId(qualId()), delimited=false)
}
else Nil
possibleTemplateStart()
Expand Down
21 changes: 7 additions & 14 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ object Scanners {
/** Are we in a `${ }` block? such that RBRACE exits back into multiline string. */
private def inMultiLineInterpolatedExpression =
currentRegion match {
case InBraces(InString(true, _)) => true
case InBraces(InString(true, _), _) => true
case _ => false
}

Expand Down Expand Up @@ -310,7 +310,7 @@ object Scanners {
dropBraces()
case RPAREN | RBRACKET =>
currentRegion match {
case InParens(prefix, outer) if prefix + 1 == lastToken => currentRegion = outer
case InParens(prefix, outer, _) if prefix + 1 == lastToken => currentRegion = outer
case _ =>
}
case STRINGLIT =>
Expand Down Expand Up @@ -645,13 +645,6 @@ object Scanners {
insert(OUTDENT, offset)
currentRegion = r.outer
case _ =>
lookAhead()
if isAfterLineEnd
&& (token == RPAREN || token == RBRACKET || token == RBRACE || token == OUTDENT)
then
() /* skip the trailing comma */
else
reset()
case END =>
if !isEndMarker then token = IDENTIFIER
case COLON =>
Expand Down Expand Up @@ -1441,7 +1434,7 @@ object Scanners {
def proposeKnownWidth(width: IndentWidth, lastToken: Token) =
if knownWidth == null then
this match
case InParens(_, _) if lastToken != LPAREN =>
case InParens(_, _, _) if lastToken != LPAREN =>
useOuterWidth()
case _ =>
knownWidth = width
Expand All @@ -1452,8 +1445,8 @@ object Scanners {

private def delimiter = this match
case _: InString => "}(in string)"
case InParens(LPAREN, _) => ")"
case InParens(LBRACKET, _) => "]"
case InParens(LPAREN, _, _) => ")"
case InParens(LBRACKET, _, _) => "]"
case _: InBraces => "}"
case _: InCase => "=>"
case _: Indented => "UNDENT"
Expand All @@ -1468,8 +1461,8 @@ object Scanners {
end Region

case class InString(multiLine: Boolean, outer: Region) extends Region
case class InParens(prefix: Token, outer: Region) extends Region
case class InBraces(outer: Region) extends Region
case class InParens(prefix: Token, outer: Region, commaSeparated: Boolean = false) extends Region
case class InBraces(outer: Region, commaSeparated: Boolean = false) extends Region
case class InCase(outer: Region) extends Region

/** A class describing an indentation region.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ object MarkupParsers {

/** xScalaPatterns ::= patterns
*/
def xScalaPatterns: List[Tree] = escapeToScala(parser.patterns(), "pattern")
def xScalaPatterns: List[Tree] = escapeToScala(parser.patterns(delimited = false), "pattern")

def reportSyntaxError(offset: Int, str: String): Unit = parser.syntaxError(str, offset)
def reportSyntaxError(str: String): Unit = {
Expand Down
36 changes: 36 additions & 0 deletions tests/neg/comma-separated-errors.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:21 ----------------------------------------------------
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
| ^
| ')' expected, but integer literal found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:26 ----------------------------------------------------
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
| ^^^
| ':' expected, but identifier found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:42 ----------------------------------------------------
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
| ^
| ')' expected, but integer literal found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:3:47 ----------------------------------------------------
3 | def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error
| ^
| ':' expected, but '=' found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:11:16 ---------------------------------------------------
11 | case Plus(4 1) => // error
| ^
| ')' expected, but integer literal found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:16 ---------------------------------------------------
12 | case Plus(4 5 6 7, 1, 2 3) => // error // error
| ^
| ')' expected, but integer literal found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:12:28 ---------------------------------------------------
12 | case Plus(4 5 6 7, 1, 2 3) => // error // error
| ^
| ')' expected, but integer literal found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:12 ---------------------------------------------------
14 | val x: A[T=Int, T=Int] = ??? // error // error
| ^
| ']' expected, but '=' found
-- [E040] Syntax Error: tests/neg/comma-separated-errors.scala:14:19 ---------------------------------------------------
14 | val x: A[T=Int, T=Int] = ??? // error // error
| ^
| ']' expected, but '=' found
15 changes: 15 additions & 0 deletions tests/neg/comma-separated-errors.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class A[T]
object o {
def foo(x: Int = 5 6, y Int = 7, z: Int 5, x = 5): Unit = () // error // error // error // error

case class Plus(a: Int, b: Int)

object Plus {
def unapply(r: Int): Plus = Plus(r - 1, 1)
}
5 match {
case Plus(4 1) => // error
case Plus(4 5 6 7, 1, 2 3) => // error // error
}
val x: A[T=Int, T=Int] = ??? // error // error
}
2 changes: 1 addition & 1 deletion tests/neg/i1679.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class A[T]
object o {
// Testing compiler crash, this test should be modified when named type argument are completely implemented
val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error
val x: A[T=Int, T=Int] = ??? // error: ']' expected, but '=' found // error: ']' expected, but '=' found
}
18 changes: 18 additions & 0 deletions tests/neg/t11900.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- Error: tests/neg/t11900.scala:44:16 ---------------------------------------------------------------------------------
44 | a => a + 1, // error: weird comma
| ^
| end of statement expected but ',' found
-- Error: tests/neg/t11900.scala:48:16 ---------------------------------------------------------------------------------
48 | println("a"), // error: weird comma
| ^
| end of statement expected but ',' found
-- Error: tests/neg/t11900.scala:52:16 ---------------------------------------------------------------------------------
52 | println("b"), // error: weird comma
| ^
| end of statement expected but ',' found
-- [E032] Syntax Error: tests/neg/t11900.scala:64:8 --------------------------------------------------------------------
64 | _*, // error
| ^
| pattern expected
|
| longer explanation available when compiling with `-explain`
Loading

0 comments on commit fc759ec

Please sign in to comment.