From 8c2e2e090bbce35acea9f1a2e1696c6bc471f3e4 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 1 Nov 2017 19:10:53 +0100 Subject: [PATCH 1/4] Fix #3332: Disallow bound variable sin pattern alternatives Disallow bound variables where the binder appears as a pattern alternative. --- compiler/src/dotty/tools/dotc/core/Mode.scala | 7 +++++-- compiler/src/dotty/tools/dotc/typer/Namer.scala | 4 ++-- compiler/src/dotty/tools/dotc/typer/Typer.scala | 6 ++++-- tests/neg/i3332.scala | 9 +++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 tests/neg/i3332.scala diff --git a/compiler/src/dotty/tools/dotc/core/Mode.scala b/compiler/src/dotty/tools/dotc/core/Mode.scala index 39e3ca21462d..5affc2cb8fd4 100644 --- a/compiler/src/dotty/tools/dotc/core/Mode.scala +++ b/compiler/src/dotty/tools/dotc/core/Mode.scala @@ -8,7 +8,7 @@ case class Mode(val bits: Int) extends AnyVal { def &~ (that: Mode) = Mode(bits & ~that.bits) def is (that: Mode) = (bits & that.bits) == that.bits - def isExpr = (this & PatternOrType) == None + def isExpr = (this & PatternOrTypeBits) == None override def toString = (0 until 31).filter(i => (bits & (1 << i)) != 0).map(modeName).mkString("Mode(", ",", ")") @@ -42,6 +42,9 @@ object Mode { /** We are looking at the arguments of a supercall */ val InSuperCall = newMode(6, "InSuperCall") + /** We are in a pattern alternative */ + val InPatternAlternative = newMode(7, "InPatternAlternative") + /** Allow GADTFlexType labelled types to have their bounds adjusted */ val GADTflexible = newMode(8, "GADTflexible") @@ -87,7 +90,7 @@ object Mode { /** Don't suppress exceptions thrown during show */ val PrintShowExceptions = newMode(18, "PrintShowExceptions") - val PatternOrType = Pattern | Type + val PatternOrTypeBits = Pattern | Type | InPatternAlternative /** We are elaborating the fully qualified name of a package clause. * In this case, identifiers should never be imported. diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 61c57858f450..36216adf06dd 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -936,10 +936,10 @@ class Namer { typer: Typer => } def typedAheadType(tree: Tree, pt: Type = WildcardType)(implicit ctx: Context): tpd.Tree = - typedAheadImpl(tree, typer.typed(_, pt)(ctx retractMode Mode.PatternOrType addMode Mode.Type)) + typedAheadImpl(tree, typer.typed(_, pt)(ctx retractMode Mode.PatternOrTypeBits addMode Mode.Type)) def typedAheadExpr(tree: Tree, pt: Type = WildcardType)(implicit ctx: Context): tpd.Tree = - typedAheadImpl(tree, typer.typed(_, pt)(ctx retractMode Mode.PatternOrType)) + typedAheadImpl(tree, typer.typed(_, pt)(ctx retractMode Mode.PatternOrTypeBits)) def typedAheadAnnotation(tree: Tree)(implicit ctx: Context): Symbol = tree match { case Apply(fn, _) => typedAheadAnnotation(fn) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index c4679416e53f..56727eca6b3a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1211,12 +1211,14 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit typed(untpd.Bind(tree.name, untpd.TypedSplice(arg)).withPos(tree.pos), arg.tpe) :: Nil) case _ => val sym = newPatternBoundSym(tree.name, body1.tpe, tree.pos) + if (ctx.mode.is(Mode.InPatternAlternative)) ctx.error("Illegal variable in pattern alternative", tree.pos) assignType(cpy.Bind(tree)(tree.name, body1), sym) } } def typedAlternative(tree: untpd.Alternative, pt: Type)(implicit ctx: Context): Alternative = track("typedAlternative") { - val trees1 = tree.trees mapconserve (typed(_, pt)) + val nestedCtx = ctx.addMode(Mode.InPatternAlternative) + val trees1 = tree.trees.mapconserve(typed(_, pt)(nestedCtx)) assignType(cpy.Alternative(tree)(trees1), trees1) } @@ -1754,7 +1756,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit Inliner.removeInlineAccessors(mdef.symbol) def typedExpr(tree: untpd.Tree, pt: Type = WildcardType)(implicit ctx: Context): Tree = - typed(tree, pt)(ctx retractMode Mode.PatternOrType) + typed(tree, pt)(ctx retractMode Mode.PatternOrTypeBits) def typedType(tree: untpd.Tree, pt: Type = WildcardType)(implicit ctx: Context): Tree = // todo: retract mode between Type and Pattern? typed(tree, pt)(ctx addMode Mode.Type) def typedPattern(tree: untpd.Tree, selType: Type = WildcardType)(implicit ctx: Context): Tree = diff --git a/tests/neg/i3332.scala b/tests/neg/i3332.scala new file mode 100644 index 000000000000..de7cd10b73fe --- /dev/null +++ b/tests/neg/i3332.scala @@ -0,0 +1,9 @@ +object Main { + def main(args: Array[String]): Unit = { + (1: Any) match { + case x: String | Int => "OK" + case (_: String) | (_: Int) => "OK" + case (s: String) | _: Int => s // error: Illegal variable in pattern alternative + } + } +} From 92dfdf79a7dea111ee6821b2ecac842fa856a75c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 1 Nov 2017 19:15:38 +0100 Subject: [PATCH 2/4] Add test case for #1612 --- tests/neg/i3332.scala | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/neg/i3332.scala b/tests/neg/i3332.scala index de7cd10b73fe..4eb7794d21ff 100644 --- a/tests/neg/i3332.scala +++ b/tests/neg/i3332.scala @@ -7,3 +7,11 @@ object Main { } } } + +// #1612 +object Test { + def g(p:(Int,Int)) = p match { + case (10,n) | (n,10) => println(n) // error // error (Illegal variable in pattern alternative) + case _ => println("nope") + } +} From 3cc6559ceb9f7c7482c3d586c3a3b71e446afbae Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 1 Nov 2017 19:19:56 +0100 Subject: [PATCH 3/4] Fix test --- tests/neg/i3332.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neg/i3332.scala b/tests/neg/i3332.scala index 4eb7794d21ff..11c558d51f59 100644 --- a/tests/neg/i3332.scala +++ b/tests/neg/i3332.scala @@ -1,7 +1,7 @@ object Main { def main(args: Array[String]): Unit = { (1: Any) match { - case x: String | Int => "OK" + case x: String | Int => "OK" // error: Illegal variable in pattern alternative case (_: String) | (_: Int) => "OK" case (s: String) | _: Int => s // error: Illegal variable in pattern alternative } From de555100826067b94f417fbd728675373d77d975 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 2 Nov 2017 15:14:41 +0100 Subject: [PATCH 4/4] Don't flag `_` as illegal variables in pattern alternatives I thought that `_` never appears as a bound variable in a bind but was wrong. We now rewrite `(_ @ P)` to `P`. --- compiler/src/dotty/tools/dotc/typer/Typer.scala | 10 +++++++--- tests/neg/i3332.scala | 4 ++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 56727eca6b3a..e8da2e58b628 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1210,9 +1210,13 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit tpd.cpy.UnApply(body1)(fn, Nil, typed(untpd.Bind(tree.name, untpd.TypedSplice(arg)).withPos(tree.pos), arg.tpe) :: Nil) case _ => - val sym = newPatternBoundSym(tree.name, body1.tpe, tree.pos) - if (ctx.mode.is(Mode.InPatternAlternative)) ctx.error("Illegal variable in pattern alternative", tree.pos) - assignType(cpy.Bind(tree)(tree.name, body1), sym) + if (tree.name == nme.WILDCARD) body1 + else { + val sym = newPatternBoundSym(tree.name, body1.tpe, tree.pos) + if (ctx.mode.is(Mode.InPatternAlternative)) + ctx.error(i"Illegal variable ${sym.name} in pattern alternative", tree.pos) + assignType(cpy.Bind(tree)(tree.name, body1), sym) + } } } diff --git a/tests/neg/i3332.scala b/tests/neg/i3332.scala index 11c558d51f59..e2964a22050e 100644 --- a/tests/neg/i3332.scala +++ b/tests/neg/i3332.scala @@ -10,8 +10,12 @@ object Main { // #1612 object Test { + case class A() def g(p:(Int,Int)) = p match { case (10,n) | (n,10) => println(n) // error // error (Illegal variable in pattern alternative) case _ => println("nope") } + def test(x: Any) = x match { + case _: String | _ @ A() => 1 + } }