Skip to content

Handle trailing commas in parser instead of scanner #14517

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.

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 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
111 changes: 56 additions & 55 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -562,19 +562,35 @@ 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 expectedEnd If set to something other than [[EMPTY]],
* assume this comma separated list must be followed by this token.
* If the parser consumes a `part` that is not followed by a comma or this expected
* token, issue a syntax error and try to recover at the next safe point.
*/
def commaSeparated[T](part: () => T, expectedEnd: Token, readFirst: Boolean = true): List[T] = {
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))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Main change here, this is moved out of the scanner, but here we no longer have to guess what lookahead token might indicate a trailing comma.

// 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, expectedEnd)
}
}
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 @@ -1391,14 +1407,7 @@ object Parsers {
else
Function(params, t)
}
def funTypeArgsRest(first: Tree, following: () => Tree) = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inlined, since this is just reading a comma-separated list.

val buf = new ListBuffer[Tree] += first
while (in.token == COMMA) {
in.nextToken()
buf += following()
}
buf.toList
}

var isValParamList = false

val t =
Expand All @@ -1414,11 +1423,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), RPAREN, readFirst = false)
case t =>
funTypeArgsRest(t, funArgType)
t :: commaSeparated(funArgType, RPAREN, readFirst = false)
}
accept(RPAREN)
if isValParamList || in.isArrow then
Expand Down Expand Up @@ -1519,7 +1527,7 @@ object Parsers {
/** FunParamClause ::= ‘(’ TypedFunParam {‘,’ TypedFunParam } ‘)’
*/
def funParamClause(): List[ValDef] =
inParens(commaSeparated(() => typedFunParam(in.offset, ident())))
inParens(commaSeparated(() => typedFunParam(in.offset, ident()), RPAREN))

def funParamClauses(): List[List[ValDef]] =
if in.token == LPAREN then funParamClause() :: funParamClauses() else Nil
Expand Down Expand Up @@ -1631,7 +1639,7 @@ object Parsers {
else
def singletonArgs(t: Tree): Tree =
if in.token == LPAREN && in.featureEnabled(Feature.dependent)
then singletonArgs(AppliedTypeTree(t, inParens(commaSeparated(singleton))))
then singletonArgs(AppliedTypeTree(t, inParens(commaSeparated(singleton, RPAREN))))
else t
singletonArgs(simpleType1())

Expand All @@ -1647,7 +1655,7 @@ object Parsers {
def simpleType1() = simpleTypeRest {
if in.token == LPAREN then
atSpan(in.offset) {
makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true)))
makeTupleOrParens(inParens(argTypes(namedOK = false, wildOK = true, RPAREN)))
}
else if in.token == LBRACE then
atSpan(in.offset) { RefinedTypeTree(EmptyTree, refinement(indentOK = false)) }
Expand Down Expand Up @@ -1731,7 +1739,7 @@ object Parsers {
* | NamedTypeArg {`,' NamedTypeArg}
* NamedTypeArg ::= id `=' Type
*/
def argTypes(namedOK: Boolean, wildOK: Boolean): List[Tree] = {
def argTypes(namedOK: Boolean, wildOK: Boolean, expectedEnd: Token): List[Tree] = {

def argType() = {
val t = typ()
Expand All @@ -1748,7 +1756,7 @@ object Parsers {
val rest =
if (in.token == COMMA) {
in.nextToken()
commaSeparated(arg)
commaSeparated(arg, expectedEnd)
}
else Nil
first :: rest
Expand All @@ -1761,7 +1769,7 @@ object Parsers {
case firstArg =>
otherArgs(firstArg, () => argType())
}
else commaSeparated(() => argType())
else commaSeparated(() => argType(), expectedEnd)
}

/** FunArgType ::= Type | `=>' Type
Expand Down Expand Up @@ -1790,7 +1798,7 @@ object Parsers {
/** TypeArgs ::= `[' Type {`,' Type} `]'
* NamedTypeArgs ::= `[' NamedTypeArg {`,' NamedTypeArg} `]'
*/
def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] = inBrackets(argTypes(namedOK, wildOK))
def typeArgs(namedOK: Boolean, wildOK: Boolean): List[Tree] = inBrackets(argTypes(namedOK, wildOK, RBRACKET))

/** Refinement ::= `{' RefineStatSeq `}'
*/
Expand Down Expand Up @@ -2154,7 +2162,7 @@ object Parsers {
var mods1 = mods
if isErased then mods1 = addModifier(mods1)
try
commaSeparated(() => binding(mods1))
commaSeparated(() => binding(mods1), RPAREN)
finally
accept(RPAREN)
else {
Expand Down Expand Up @@ -2384,7 +2392,7 @@ object Parsers {
/** ExprsInParens ::= ExprInParens {`,' ExprInParens}
*/
def exprsInParensOpt(): List[Tree] =
if (in.token == RPAREN) Nil else commaSeparated(exprInParens)
if (in.token == RPAREN) Nil else commaSeparated(exprInParens, RPAREN)

/** ParArgumentExprs ::= `(' [‘using’] [ExprsInParens] `)'
* | `(' [ExprsInParens `,'] PostfixExpr `*' ')'
Expand All @@ -2394,9 +2402,9 @@ object Parsers {
(Nil, false)
else if isIdent(nme.using) then
in.nextToken()
(commaSeparated(argumentExpr), true)
(commaSeparated(argumentExpr, RPAREN), true)
else
(commaSeparated(argumentExpr), false)
(commaSeparated(argumentExpr, RPAREN), false)
}

/** ArgumentExprs ::= ParArgumentExprs
Expand Down Expand Up @@ -2540,7 +2548,7 @@ object Parsers {
if (leading == LBRACE || in.token == CASE)
enumerators()
else {
val pats = patternsOpt()
val pats = patternsOpt(EMPTY)
val pat =
if (in.token == RPAREN || pats.length > 1) {
wrappedEnums = false
Expand Down Expand Up @@ -2732,7 +2740,7 @@ object Parsers {
case USCORE =>
wildcardIdent()
case LPAREN =>
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt())) }
atSpan(in.offset) { makeTupleOrParens(inParens(patternsOpt(RPAREN))) }
case QUOTE =>
simpleExpr(Location.InPattern)
case XMLSTART =>
Expand Down Expand Up @@ -2768,17 +2776,17 @@ object Parsers {

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

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

/** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’
* | ‘(’ [Patterns ‘,’] PatVar ‘*’ ‘)’
*/
def argumentPatterns(): List[Tree] =
inParens(patternsOpt(Location.InPatternArgs))
inParens(patternsOpt(RPAREN, Location.InPatternArgs))

/* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */

Expand Down Expand Up @@ -2959,7 +2967,7 @@ object Parsers {
TypeDef(name, lambdaAbstract(hkparams, bounds)).withMods(mods)
}
}
commaSeparated(() => typeParam())
commaSeparated(() => typeParam(), RBRACKET)
}

def typeParamClauseOpt(ownerKind: ParamOwner): List[TypeDef] =
Expand All @@ -2968,7 +2976,7 @@ object Parsers {
/** ContextTypes ::= FunArgType {‘,’ FunArgType}
*/
def contextTypes(ofClass: Boolean, nparams: Int, impliedMods: Modifiers): List[ValDef] =
val tps = commaSeparated(funArgType)
val tps = commaSeparated(funArgType, RPAREN)
var counter = nparams
def nextIdx = { counter += 1; counter }
val paramFlags = if ofClass then Private | Local | ParamAccessor else Param
Expand Down Expand Up @@ -3072,7 +3080,7 @@ object Parsers {
!impliedMods.is(Given)
|| startParamTokens.contains(in.token)
|| isIdent && (in.name == nme.inline || in.lookahead.isColon())
if isParams then commaSeparated(() => param())
if isParams then commaSeparated(() => param(), RPAREN)
else contextTypes(ofClass, nparams, impliedMods)
checkVarArgsRules(clause)
clause
Expand Down Expand Up @@ -3121,7 +3129,7 @@ object Parsers {
*/
def importClause(leading: Token, mkTree: ImportConstr): List[Tree] = {
val offset = accept(leading)
commaSeparated(importExpr(mkTree)) match {
commaSeparated(importExpr(mkTree), EMPTY) match {
case t :: rest =>
// The first import should start at the start offset of the keyword.
val firstPos =
Expand Down Expand Up @@ -3198,9 +3206,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 @@ -3210,13 +3218,6 @@ object Parsers {
if !idOK then syntaxError(i"named imports cannot follow wildcard imports")
namedSelector(termIdent())
}
val rest =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another comma-separated list factored out.

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 @@ -3234,7 +3235,7 @@ object Parsers {
case GIVEN =>
mkTree(qual, givenSelector() :: Nil)
case LBRACE =>
mkTree(qual, inBraces(importSelectors(idOK = true)))
mkTree(qual, inBraces(commaSeparated(importSelector(idOK = true), RBRACE)))
case _ =>
if isIdent(nme.raw.STAR) then
mkTree(qual, wildcardSelector() :: Nil)
Expand Down Expand Up @@ -3291,7 +3292,7 @@ object Parsers {
var lhs = first match {
case id: Ident if in.token == COMMA =>
in.nextToken()
id :: commaSeparated(() => termIdent())
id :: commaSeparated(() => termIdent(), EMPTY)
case _ =>
first :: Nil
}
Expand Down Expand Up @@ -3562,7 +3563,7 @@ object Parsers {
val id = termIdent()
if (in.token == COMMA) {
in.nextToken()
val ids = commaSeparated(() => termIdent())
val ids = commaSeparated(() => termIdent(), EMPTY)
PatDef(mods1, id :: ids, TypeTree(), EmptyTree)
}
else {
Expand Down Expand Up @@ -3766,7 +3767,7 @@ object Parsers {
val derived =
if (isIdent(nme.derives)) {
in.nextToken()
tokenSeparated(COMMA, () => convertToTypeId(qualId()))
commaSeparated(() => convertToTypeId(qualId()), EMPTY)
}
else Nil
possibleTemplateStart()
Expand Down
7 changes: 0 additions & 7 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -660,13 +660,6 @@ object Scanners {
insert(OUTDENT, offset)
currentRegion = r.outer
case _ =>
lookAhead()
if isAfterLineEnd
&& (token == RPAREN || token == RBRACKET || token == RBRACE || token == OUTDENT)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part that was moved to the parser.

then
() /* skip the trailing comma */
else
reset()
case END =>
if !isEndMarker then token = IDENTIFIER
case COLON =>
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
}
Loading