From 4639991e15f4425319b7599b6b0d9c89d212c608 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 20 Apr 2020 15:04:21 +0200 Subject: [PATCH 1/2] Fix #8715: enforce syntax for _* (alternate version) Previously we didn't check that _* is indeed the last argument, checking for ")" is not enough, as ")" may be the closing parenthesis of a nested pattern. --- community-build/community-projects/stdLib213 | 2 +- .../dotty/tools/dotc/parsing/Parsers.scala | 83 ++++++++++--------- .../dotty/tools/vulpix/ParallelTesting.scala | 2 +- tests/neg/i7972.scala | 10 +-- tests/neg/i8715.scala | 2 + tests/neg/t5702-neg-bad-and-wild.scala | 29 +++++++ tests/pos-scala2/i8715b.scala | 4 + .../untried/neg/t5702-neg-bad-and-wild.scala | 29 ------- 8 files changed, 88 insertions(+), 73 deletions(-) create mode 100644 tests/neg/i8715.scala create mode 100644 tests/neg/t5702-neg-bad-and-wild.scala create mode 100644 tests/pos-scala2/i8715b.scala delete mode 100644 tests/untried/neg/t5702-neg-bad-and-wild.scala diff --git a/community-build/community-projects/stdLib213 b/community-build/community-projects/stdLib213 index a6dcbd046597..00c8764bf289 160000 --- a/community-build/community-projects/stdLib213 +++ b/community-build/community-projects/stdLib213 @@ -1 +1 @@ -Subproject commit a6dcbd0465974543bb4dbe960300686a71383631 +Subproject commit 00c8764bf289396c0c6f8496c907f9a37f0415cb diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 9dbb2a7efbb9..f8ffffc8e106 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -48,9 +48,13 @@ object Parsers { def nonePositive: Boolean = parCounts forall (_ <= 0) } - @sharable object Location extends Enumeration { - val InParens, InBlock, InPattern, ElseWhere: Value = Value - } + enum Location(val inParens: Boolean, val inPattern: Boolean, val inArgs: Boolean): + case InParens extends Location(true, false, false) + case InArgs extends Location(true, false, true) + case InPattern extends Location(false, true, false) + case InPatternArgs extends Location(false, true, true) // InParens not true, since it might be an alternative + case InBlock extends Location(false, false, false) + case ElseWhere extends Location(false, false, false) @sharable object ParamOwner extends Enumeration { val Class, Type, TypeParam, Def: Value = Value @@ -1754,9 +1758,9 @@ object Parsers { else TypeTree().withSpan(Span(in.lastOffset)) } - def typeDependingOn(location: Location.Value): Tree = - if (location == Location.InParens) typ() - else if (location == Location.InPattern) refinedType() + def typeDependingOn(location: Location): Tree = + if location.inParens then typ() + else if location.inPattern then refinedType() else infixType() /* ----------- EXPRESSIONS ------------------------------------------------ */ @@ -1843,7 +1847,7 @@ object Parsers { def subExpr() = subPart(expr) - def expr(location: Location.Value): Tree = { + def expr(location: Location): Tree = { val start = in.offset def isSpecialClosureStart = val lookahead = in.LookaheadScanner() @@ -1876,7 +1880,7 @@ object Parsers { } } - def expr1(location: Location.Value = Location.ElseWhere): Tree = in.token match + def expr1(location: Location = Location.ElseWhere): Tree = in.token match case IF => in.endMarkerScope(IF) { ifExpr(in.offset, If) } case WHILE => @@ -1989,11 +1993,13 @@ object Parsers { else expr1Rest(postfixExpr(), location) end expr1 - def expr1Rest(t: Tree, location: Location.Value): Tree = in.token match + def expr1Rest(t: Tree, location: Location): Tree = in.token match case EQUALS => t match case Ident(_) | Select(_, _) | Apply(_, _) => - atSpan(startOffset(t), in.skipToken()) { Assign(t, subExpr()) } + atSpan(startOffset(t), in.skipToken()) { + Assign(t, subPart(() => expr(location))) + } case _ => t case COLON => @@ -2003,24 +2009,29 @@ object Parsers { t end expr1Rest - def ascription(t: Tree, location: Location.Value): Tree = atSpan(startOffset(t)) { + def ascription(t: Tree, location: Location): Tree = atSpan(startOffset(t)) { in.token match { case USCORE => val uscoreStart = in.skipToken() - if (isIdent(nme.raw.STAR)) { + if isIdent(nme.raw.STAR) then in.nextToken() - if (in.token != RPAREN) syntaxError(SeqWildcardPatternPos(), uscoreStart) + if !(location.inArgs && in.token == RPAREN) then + if opStack.nonEmpty + ctx.errorOrMigrationWarning( + em"""`_*` can be used only for last argument of method application. + |It is no longer allowed in operands of infix operations.""", + in.sourcePos(uscoreStart)) + else + syntaxError(SeqWildcardPatternPos(), uscoreStart) Typed(t, atSpan(uscoreStart) { Ident(tpnme.WILDCARD_STAR) }) - } - else { + else syntaxErrorOrIncomplete(IncorrectRepeatedParameterSyntax()) t - } - case AT if location != Location.InPattern => + case AT if !location.inPattern => annotations().foldLeft(t)(Annotated) case _ => val tpt = typeDependingOn(location) - if (isWildcard(t) && location != Location.InPattern) { + if (isWildcard(t) && !location.inPattern) { val vd :: rest = placeholderParams placeholderParams = cpy.ValDef(vd)(tpt = tpt).withSpan(vd.span.union(tpt.span)) :: rest @@ -2063,7 +2074,7 @@ object Parsers { * | `_' * Bindings ::= `(' [[‘using’] [‘erased’] Binding {`,' Binding}] `)' */ - def funParams(mods: Modifiers, location: Location.Value): List[Tree] = + def funParams(mods: Modifiers, location: Location): List[Tree] = if in.token == LPAREN then in.nextToken() if in.token == RPAREN then @@ -2117,10 +2128,10 @@ object Parsers { /** Expr ::= [‘implicit’] FunParams `=>' Expr * BlockResult ::= implicit id [`:' InfixType] `=>' Block // Scala2 only */ - def closure(start: Int, location: Location.Value, implicitMods: Modifiers): Tree = + def closure(start: Int, location: Location, implicitMods: Modifiers): Tree = closureRest(start, location, funParams(implicitMods, location)) - def closureRest(start: Int, location: Location.Value, params: List[Tree]): Tree = + def closureRest(start: Int, location: Location, params: List[Tree]): Tree = atSpan(start, in.offset) { if in.token == CTXARROW then in.nextToken() else accept(ARROW) Function(params, if (location == Location.InBlock) block() else expr()) @@ -2295,10 +2306,9 @@ object Parsers { if args._2 then res.setUsingApply() res - val argumentExpr: () => Tree = () => exprInParens() match { + val argumentExpr: () => Tree = () => expr(Location.InArgs) match case arg @ Assign(Ident(id), rhs) => cpy.NamedArg(arg)(id, rhs) case arg => arg - } /** ArgumentExprss ::= {ArgumentExprs} */ @@ -2535,21 +2545,20 @@ object Parsers { /** Pattern ::= Pattern1 { `|' Pattern1 } */ - val pattern: () => Tree = () => { - val pat = pattern1() + def pattern(location: Location = Location.InPattern): Tree = + val pat = pattern1(location) if (isIdent(nme.raw.BAR)) - atSpan(startOffset(pat)) { Alternative(pat :: patternAlts()) } + atSpan(startOffset(pat)) { Alternative(pat :: patternAlts(location)) } else pat - } - def patternAlts(): List[Tree] = - if (isIdent(nme.raw.BAR)) { in.nextToken(); pattern1() :: patternAlts() } + def patternAlts(location: Location): List[Tree] = + if (isIdent(nme.raw.BAR)) { in.nextToken(); pattern1(location) :: patternAlts(location) } else Nil /** Pattern1 ::= Pattern2 [Ascription] * | ‘given’ PatVar ‘:’ RefinedType */ - def pattern1(): Tree = + def pattern1(location: Location = Location.InPattern): Tree = if (in.token == GIVEN) { val givenMod = atSpan(in.skipToken())(Mod.Given()) atSpan(in.offset) { @@ -2558,7 +2567,7 @@ object Parsers { val name = in.name in.nextToken() accept(COLON) - val typed = ascription(Ident(nme.WILDCARD), Location.InPattern) + val typed = ascription(Ident(nme.WILDCARD), location) Bind(name, typed).withMods(addMod(Modifiers(), givenMod)) case _ => syntaxErrorOrIncomplete("pattern variable expected") @@ -2570,7 +2579,7 @@ object Parsers { val p = pattern2() if (in.token == COLON) { in.nextToken() - ascription(p, Location.InPattern) + ascription(p, location) } else p } @@ -2661,17 +2670,17 @@ object Parsers { /** Patterns ::= Pattern [`,' Pattern] */ - def patterns(): List[Tree] = commaSeparated(pattern) - - def patternsOpt(): List[Tree] = - if (in.token == RPAREN) Nil else patterns() + def patterns(location: Location = Location.InPattern): List[Tree] = + commaSeparated(() => pattern(location)) + def patternsOpt(location: Location = Location.InPattern): List[Tree] = + if (in.token == RPAREN) Nil else patterns(location) /** ArgumentPatterns ::= ‘(’ [Patterns] ‘)’ * | ‘(’ [Patterns ‘,’] Pattern2 ‘:’ ‘_’ ‘*’ ‘)’ */ def argumentPatterns(): List[Tree] = - inParens(patternsOpt()) + inParens(patternsOpt(Location.InPatternArgs)) /* -------- MODIFIERS and ANNOTATIONS ------------------------------------------- */ diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 2417294c50ed..995033b4c475 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -627,7 +627,7 @@ trait ParallelTesting extends RunnerOrchestration { self => lazy val actualErrors = reporters.foldLeft(0)(_ + _.errorCount) def hasMissingAnnotations = getMissingExpectedErrors(errorMap, reporters.iterator.flatMap(_.errors)) def showErrors = "-> following the errors:\n" + - reporters.flatMap(_.allErrors.map(e => e.pos.toString + ": " + e.message)).mkString(start = "at ", sep = "\n at ", end = "") + reporters.flatMap(_.allErrors.map(e => e.pos.line.toString + ": " + e.message)).mkString(start = "at ", sep = "\n at ", end = "") if (compilerCrashed) Some(s"Compiler crashed when compiling: ${testSource.title}") else if (actualErrors == 0) Some(s"\nNo errors found when compiling neg test $testSource") diff --git a/tests/neg/i7972.scala b/tests/neg/i7972.scala index 1003e30f460e..abe3185d6b03 100644 --- a/tests/neg/i7972.scala +++ b/tests/neg/i7972.scala @@ -1,18 +1,18 @@ object O { - def m1(a: Int*) = (a: _*) // error: Cannot return repeated parameter type Int* + def m1(a: Int*) = (a: _*) // error // error: Cannot return repeated parameter type Int* def m2(a: Int*) = { // error: Cannot return repeated parameter type Int* - val b = (a: _*) // error: Cannot return repeated parameter type Int* + val b = (a: _*) // error // error: Cannot return repeated parameter type Int* b } def m3(a: Int*): Any = { - val b = (a: _*) // error: Cannot return repeated parameter type Int* + val b = (a: _*) // error // error: Cannot return repeated parameter type Int* b } - def m4(a: 2*) = (a: _*) // error: Cannot return repeated parameter type Int* + def m4(a: 2*) = (a: _*) // error // error: Cannot return repeated parameter type Int* } class O(a: Int*) { - val m = (a: _*) // error: Cannot return repeated parameter type Int* + val m = (a: _*) // error // error: Cannot return repeated parameter type Int* } diff --git a/tests/neg/i8715.scala b/tests/neg/i8715.scala new file mode 100644 index 000000000000..aedb32cffdcb --- /dev/null +++ b/tests/neg/i8715.scala @@ -0,0 +1,2 @@ +@main +def Test = List(42) match { case List(xs @ (ys: _*)) => xs } // error diff --git a/tests/neg/t5702-neg-bad-and-wild.scala b/tests/neg/t5702-neg-bad-and-wild.scala new file mode 100644 index 000000000000..7566828256a8 --- /dev/null +++ b/tests/neg/t5702-neg-bad-and-wild.scala @@ -0,0 +1,29 @@ + +object Test { + case class K(i: Int) + + def main(args: Array[String]) = { + val k = new K(9) + val is = List(1,2,3) + + is match { + case List(1, _*,) => // error // error // error: bad use of _* (a sequence pattern must be the last pattern) + // illegal start of simple pattern + case List(1, _*3,) => // error // error: illegal start of simple pattern + //case List(1, _*3:) => // poor recovery by parens + case List(1, x*) => // error: use _* to match a sequence + case List(x*, 1) => // error: trailing * is not a valid pattern + case (1, x*) => // error: trailing * is not a valid pattern + case (1, x: _*) => // error: bad use of _* (sequence pattern not allowed) + } + +// good syntax, bad semantics, detected by typer +//gowild.scala:14: error: star patterns must correspond with varargs parameters + val K(x @ _*) = k + val K(ns @ _*, x) = k // error: bad use of _* (a sequence pattern must be the last pattern) + val (b, _ : _* ) = (5,6) // error: bad use of _* (sequence pattern not allowed) +// no longer complains +//bad-and-wild.scala:15: error: ')' expected but '}' found. + } +} + diff --git a/tests/pos-scala2/i8715b.scala b/tests/pos-scala2/i8715b.scala new file mode 100644 index 000000000000..18f053806496 --- /dev/null +++ b/tests/pos-scala2/i8715b.scala @@ -0,0 +1,4 @@ +// from stdlib +class Test { + def printf(text: String, args: Any*): Unit = { System.out.print(text format (args : _*)) } +} \ No newline at end of file diff --git a/tests/untried/neg/t5702-neg-bad-and-wild.scala b/tests/untried/neg/t5702-neg-bad-and-wild.scala deleted file mode 100644 index aadda37da7c3..000000000000 --- a/tests/untried/neg/t5702-neg-bad-and-wild.scala +++ /dev/null @@ -1,29 +0,0 @@ - -object Test { - case class K(i: Int) - - def main(args: Array[String]) { - val k = new K(9) - val is = List(1,2,3) - - is match { - case List(1, _*,) => // bad use of _* (a sequence pattern must be the last pattern) - // illegal start of simple pattern - case List(1, _*3,) => // illegal start of simple pattern - //case List(1, _*3:) => // poor recovery by parens - case List(1, x*) => // use _* to match a sequence - case List(x*, 1) => // trailing * is not a valid pattern - case (1, x*) => // trailing * is not a valid pattern - case (1, x@_*) => // bad use of _* (sequence pattern not allowed) - } - -// good syntax, bad semantics, detected by typer -//gowild.scala:14: error: star patterns must correspond with varargs parameters - val K(is @ _*) = k - val K(ns @ _*, x) = k // bad use of _* (a sequence pattern must be the last pattern) - val (b, _ * ) = (5,6) // bad use of _* (sequence pattern not allowed) -// no longer complains -//bad-and-wild.scala:15: error: ')' expected but '}' found. - } -} - From 7a09b822edbbaab0a85af04c55284078b044ccd2 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 20 Apr 2020 21:24:37 +0200 Subject: [PATCH 2/2] Fix location propagation in assignments --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index f8ffffc8e106..9cc072e92226 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1998,7 +1998,8 @@ object Parsers { t match case Ident(_) | Select(_, _) | Apply(_, _) => atSpan(startOffset(t), in.skipToken()) { - Assign(t, subPart(() => expr(location))) + val loc = if location.inArgs then location else Location.ElseWhere + Assign(t, subPart(() => expr(loc))) } case _ => t