From 861ac128d798748382746288daf3024c69029f53 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 30 Apr 2018 12:35:39 +0200 Subject: [PATCH 1/5] Refactor parser code that rejects forbidden wildcards The new API is required for all use cases in this PR. --- .../dotty/tools/dotc/parsing/Parsers.scala | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index edaf6850d8cf..714bcf73765f 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -41,6 +41,25 @@ object Parsers { def nonePositive: Boolean = parCounts forall (_ <= 0) } + /** Checks whether `t` is a wildcard type. + * If it is, returns the [[Position]] where the wildcard occurs. + */ + @tailrec + final def findWildcardType(t: Tree): Option[Position] = t match { + case TypeBoundsTree(_, _) => Some(t.pos) + case Parens(t1) => findWildcardType(t1) + case Annotated(t1, _) => findWildcardType(t1) + case _ => None + } + + def rejectWildcard(t: Tree, report: Position => Unit, fallbackTree: Tree): Tree = + findWildcardType(t) match { + case Some(wildcardPos) => + report(wildcardPos) + fallbackTree + case None => t + } + @sharable object Location extends Enumeration { val InParens, InBlock, InPattern, ElseWhere = Value } @@ -709,15 +728,7 @@ object Parsers { /** Same as [[typ]], but if this results in a wildcard it emits a syntax error and * returns a tree for type `Any` instead. */ - def toplevelTyp(): Tree = { - val t = typ() - findWildcardType(t) match { - case Some(wildcardPos) => - syntaxError(UnboundWildcardType(), wildcardPos) - scalaAny - case None => t - } - } + def toplevelTyp(): Tree = checkWildcard(typ()) /** Type ::= [FunArgMods] FunArgTypes `=>' Type * | HkTypeParamClause `->' Type @@ -917,7 +928,7 @@ object Parsers { else Nil first :: rest } - def typParser() = if (wildOK) typ() else toplevelTyp() + def typParser() = checkWildcard(typ(), wildOK) if (namedOK && in.token == IDENTIFIER) typParser() match { case Ident(name) if in.token == EQUALS => @@ -1001,16 +1012,8 @@ object Parsers { else if (location == Location.InPattern) refinedType() else infixType() - /** Checks whether `t` is a wildcard type. - * If it is, returns the [[Position]] where the wildcard occurs. - */ - @tailrec - private final def findWildcardType(t: Tree): Option[Position] = t match { - case TypeBoundsTree(_, _) => Some(t.pos) - case Parens(t1) => findWildcardType(t1) - case Annotated(t1, _) => findWildcardType(t1) - case _ => None - } + def checkWildcard(t: Tree, wildOK: Boolean = false, fallbackTree: Tree = scalaAny): Tree = + if (wildOK) t else rejectWildcard(t, syntaxError(UnboundWildcardType(), _), fallbackTree) /* ----------- EXPRESSIONS ------------------------------------------------ */ From 966dbb4cd172fc946f6601d87d035541301f201c Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Fri, 27 Apr 2018 20:27:55 +0200 Subject: [PATCH 2/5] Fix #4373: reject wildcard types in syntactically invalid positions Fix #4373. The testcase is from examples in #4373, variations and tests for types with wildcards. I'm unsure for a couple cases. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 7 ++++-- .../dotty/tools/dotc/parsing/Parsers.scala | 10 ++++---- tests/neg/i4373.scala | 23 +++++++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 tests/neg/i4373.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index a7586f876435..ad256c8600a9 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -892,6 +892,9 @@ object desugar { .withMods(mods) .withPos(original.pos.withPoint(named.pos.start)) + def checkWildcard(t: Tree)(implicit ctx: Context): Tree = + parsing.Parsers.rejectWildcard(t, ctx.error(UnboundWildcardType(), _), scalaAny) + /** Main desugaring method */ def apply(tree: Tree)(implicit ctx: Context): Tree = { @@ -1114,8 +1117,8 @@ object desugar { Apply(Select(Apply(Ident(nme.StringContext), strs), id), elems) case InfixOp(l, op, r) => if (ctx.mode is Mode.Type) - if (!op.isBackquoted && op.name == tpnme.raw.AMP) AndTypeTree(l, r) // l & r - else if (!op.isBackquoted && op.name == tpnme.raw.BAR) OrTypeTree(l, r) // l | r + if (!op.isBackquoted && op.name == tpnme.raw.AMP) AndTypeTree(checkWildcard(l), checkWildcard(r)) // l & r + else if (!op.isBackquoted && op.name == tpnme.raw.BAR) OrTypeTree(checkWildcard(l), checkWildcard(r)) // l | r else AppliedTypeTree(op, l :: r :: Nil) // op[l, r] else { assert(ctx.mode is Mode.Pattern) // expressions are handled separately by `binop` diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 714bcf73765f..47a9df7c4e38 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -795,7 +795,7 @@ object Parsers { val start = in.offset val tparams = typeParamClause(ParamOwner.TypeParam) if (in.token == ARROW) - atPos(start, in.skipToken())(LambdaTypeTree(tparams, typ())) + atPos(start, in.skipToken())(LambdaTypeTree(tparams, toplevelTyp())) else { accept(ARROW); typ() } } else infixType() @@ -833,7 +833,7 @@ object Parsers { def refinedTypeRest(t: Tree): Tree = { newLineOptWhenFollowedBy(LBRACE) - if (in.token == LBRACE) refinedTypeRest(atPos(startOffset(t)) { RefinedTypeTree(t, refinement()) }) + if (in.token == LBRACE) refinedTypeRest(atPos(startOffset(t)) { RefinedTypeTree(checkWildcard(t), refinement()) }) else t } @@ -897,7 +897,7 @@ object Parsers { private def simpleTypeRest(t: Tree): Tree = in.token match { case HASH => simpleTypeRest(typeProjection(t)) case LBRACKET => simpleTypeRest(atPos(startOffset(t)) { - AppliedTypeTree(t, typeArgs(namedOK = false, wildOK = true)) }) + AppliedTypeTree(checkWildcard(t), typeArgs(namedOK = false, wildOK = true)) }) case _ => t } @@ -2151,7 +2151,7 @@ object Parsers { in.token match { case EQUALS => in.nextToken() - TypeDef(name, lambdaAbstract(tparams, typ())).withMods(mods).setComment(in.getDocComment(start)) + TypeDef(name, lambdaAbstract(tparams, toplevelTyp())).withMods(mods).setComment(in.getDocComment(start)) case SUPERTYPE | SUBTYPE | SEMI | NEWLINE | NEWLINES | COMMA | RBRACE | EOF => TypeDef(name, lambdaAbstract(tparams, typeBounds())).withMods(mods).setComment(in.getDocComment(start)) case _ => @@ -2279,7 +2279,7 @@ object Parsers { /** ConstrApp ::= SimpleType {ParArgumentExprs} */ val constrApp = () => { - val t = annotType() + val t = checkWildcard(annotType()) if (in.token == LPAREN) parArgumentExprss(wrapNew(t)) else t } diff --git a/tests/neg/i4373.scala b/tests/neg/i4373.scala new file mode 100644 index 000000000000..349985521856 --- /dev/null +++ b/tests/neg/i4373.scala @@ -0,0 +1,23 @@ +trait Base +trait TypeConstr[X] + +class X1[A >: _ | X1[_]] // error +class X2[A >: _ & X2[_]] // error +class X3[A >: X3[_] | _] // error +class X4[A >: X4[_] & _] // error + +class A1 extends _ // error +class A2 extends _ with _ // error // error +class A3 extends Base with _ // error +class A4 extends _ with Base // error + +object Test { + type T1 = _ // error + type T2 = _[Int] // error + type T3 = _ { type S } // error + type T4 = [X] => _ // error + + // Open questions: + type T5 = TypeConstr[_ { type S }] // error + type T5 = TypeConstr[_[Int]] // error +} From f5aeb28f36a82eff7036debc9fcf604a4b0fba74 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 30 Apr 2018 19:43:47 +0200 Subject: [PATCH 3/5] Avoid cascade type errors from error-correction-generated code Without this commit, the testcase produces additional spurious errors: ```scala -- [E104] Syntax Error: tests/neg/i4373.scala:10:0 ----------------------------- 10 |class A2 extends _ with _ // error // error |^ |class Any is not a trait longer explanation available when compiling with `-explain` -- [E104] Syntax Error: tests/neg/i4373.scala:11:21 ---------------------------- 11 |class A3 extends Base with _ // error | ^ | class Any is not a trait longer explanation available when compiling with `-explain` -- Error: tests/neg/i4373.scala:12:24 ------------------------------------------ 12 |class A4 extends _ with Base // error | ^^^^ |illegal trait inheritance: superclass Any does not derive from trait Base's superclass Object ``` These errors make absolutely no sense for a user, since they refer to code the user didn't write, but that was generated by the compiler's error correction mechanism. I haven't checked if we could use `Ident(nme.ERROR)` everywhere. I have also tried using `EmptyTree` and `EmptyTree.withPos(wildcardPos)` and `EmptyTree.clone.withPos(wildcardPos)` as fallback trees, but all of them failed in some way. --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 47a9df7c4e38..c159db8c0526 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -56,7 +56,7 @@ object Parsers { findWildcardType(t) match { case Some(wildcardPos) => report(wildcardPos) - fallbackTree + fallbackTree.withPos(wildcardPos) case None => t } @@ -2279,7 +2279,8 @@ object Parsers { /** ConstrApp ::= SimpleType {ParArgumentExprs} */ val constrApp = () => { - val t = checkWildcard(annotType()) + // Using Ident(nme.ERROR) to avoid causing cascade errors on non-user-written code + val t = checkWildcard(annotType(), fallbackTree = Ident(nme.ERROR)) if (in.token == LPAREN) parArgumentExprss(wrapNew(t)) else t } From d968bb3b5c923f6ad9038c74ae7350f2870d747e Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 30 Apr 2018 19:51:16 +0200 Subject: [PATCH 4/5] Remove again call to withPos that seems redundant in these tests --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index c159db8c0526..fbbf44bfa9b1 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -56,7 +56,7 @@ object Parsers { findWildcardType(t) match { case Some(wildcardPos) => report(wildcardPos) - fallbackTree.withPos(wildcardPos) + fallbackTree case None => t } From 06b121e3528c0b4de792f29a01fcbf88c8f7b818 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 30 Apr 2018 21:25:51 +0200 Subject: [PATCH 5/5] Produce AndTypeTree and OrTypeTree directly in Parsers TODO: make checkWildcard private to Parsers again. --- .../src/dotty/tools/dotc/ast/Desugar.scala | 7 +------ .../src/dotty/tools/dotc/parsing/Parsers.scala | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index ad256c8600a9..31ba7ae10b95 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -892,9 +892,6 @@ object desugar { .withMods(mods) .withPos(original.pos.withPoint(named.pos.start)) - def checkWildcard(t: Tree)(implicit ctx: Context): Tree = - parsing.Parsers.rejectWildcard(t, ctx.error(UnboundWildcardType(), _), scalaAny) - /** Main desugaring method */ def apply(tree: Tree)(implicit ctx: Context): Tree = { @@ -1117,9 +1114,7 @@ object desugar { Apply(Select(Apply(Ident(nme.StringContext), strs), id), elems) case InfixOp(l, op, r) => if (ctx.mode is Mode.Type) - if (!op.isBackquoted && op.name == tpnme.raw.AMP) AndTypeTree(checkWildcard(l), checkWildcard(r)) // l & r - else if (!op.isBackquoted && op.name == tpnme.raw.BAR) OrTypeTree(checkWildcard(l), checkWildcard(r)) // l | r - else AppliedTypeTree(op, l :: r :: Nil) // op[l, r] + AppliedTypeTree(op, l :: r :: Nil) // op[l, r] else { assert(ctx.mode is Mode.Pattern) // expressions are handled separately by `binop` Apply(op, l :: r :: Nil) // op(l, r) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index fbbf44bfa9b1..92a2a650d028 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -471,7 +471,7 @@ object Parsers { if (isLeftAssoc(op1) != op2LeftAssoc) syntaxError(MixedLeftAndRightAssociativeOps(op1, op2, op2LeftAssoc), offset) - def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean, op2: Name): Tree = { + def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean, op2: Name, isType: Boolean): Tree = { if (opStack != base && precedence(opStack.head.operator.name) == prec) checkAssoc(opStack.head.offset, opStack.head.operator.name, op2, leftAssoc) def recur(top: Tree): Tree = { @@ -483,7 +483,15 @@ object Parsers { opStack = opStack.tail recur { atPos(opInfo.operator.pos union opInfo.operand.pos union top.pos) { - InfixOp(opInfo.operand, opInfo.operator, top) + val op = opInfo.operator + val l = opInfo.operand + val r = top + if (isType && !op.isBackquoted && op.name == tpnme.raw.BAR) { + OrTypeTree(checkWildcard(l), checkWildcard(r)) + } else if (isType && !op.isBackquoted && op.name == tpnme.raw.AMP) { + AndTypeTree(checkWildcard(l), checkWildcard(r)) + } else + InfixOp(l, op, r) } } } @@ -507,20 +515,20 @@ object Parsers { var top = first while (isIdent && isOperator) { val op = if (isType) typeIdent() else termIdent() - top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name), op.name) + top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name), op.name, isType) opStack = OpInfo(top, op, in.offset) :: opStack newLineOptWhenFollowing(canStartOperand) if (maybePostfix && !canStartOperand(in.token)) { val topInfo = opStack.head opStack = opStack.tail - val od = reduceStack(base, topInfo.operand, 0, true, in.name) + val od = reduceStack(base, topInfo.operand, 0, true, in.name, isType) return atPos(startOffset(od), topInfo.offset) { PostfixOp(od, topInfo.operator) } } top = operand() } - reduceStack(base, top, 0, true, in.name) + reduceStack(base, top, 0, true, in.name, isType) } /* -------- IDENTIFIERS AND LITERALS ------------------------------------------- */