diff options
| -rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala | 80 | ||||
| -rw-r--r-- | src/reflect/scala/reflect/internal/TreeInfo.scala | 2 | ||||
| -rw-r--r-- | test/files/neg/macro-invalidsig-context-bounds.check | 8 | ||||
| -rw-r--r-- | test/files/neg/macro-invalidsig-implicit-params.check | 8 | ||||
| -rw-r--r-- | test/files/neg/newpat_unreachable.check | 27 | ||||
| -rw-r--r-- | test/files/neg/newpat_unreachable.flags | 1 | ||||
| -rw-r--r-- | test/files/neg/newpat_unreachable.scala | 29 | ||||
| -rw-r--r-- | test/files/neg/pat_unreachable.check | 8 | ||||
| -rw-r--r-- | test/files/neg/pat_unreachable.scala | 8 | ||||
| -rw-r--r-- | test/files/neg/t6048.check | 7 |
10 files changed, 155 insertions, 23 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala index 1b502025c2..08c78706fe 100644 --- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala +++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala @@ -198,6 +198,69 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL import typer.{typed, context, silent, reallyExists} // import typer.infer.containsUnchecked + // Why is it so difficult to say "here's a name and a context, give me any + // matching symbol in scope" ? I am sure this code is wrong, but attempts to + // use the scopes of the contexts in the enclosing context chain discover + // nothing. How to associate a name with a symbol would would be a wonderful + // linkage for which to establish a canonical acquisition mechanism. + def matchingSymbolInScope(pat: Tree): Symbol = { + def declarationOfName(tpe: Type, name: Name): Symbol = tpe match { + case PolyType(tparams, restpe) => tparams find (_.name == name) getOrElse declarationOfName(restpe, name) + case MethodType(params, restpe) => params find (_.name == name) getOrElse declarationOfName(restpe, name) + case ClassInfoType(_, _, clazz) => clazz.rawInfo member name + case _ => NoSymbol + } + pat match { + case Bind(name, _) => + context.enclosingContextChain.foldLeft(NoSymbol: Symbol)((res, ctx) => + res orElse declarationOfName(ctx.owner.rawInfo, name)) + case _ => NoSymbol + } + } + + // Issue better warnings than "unreachable code" when people mis-use + // variable patterns thinking they bind to existing identifiers. + // + // Possible TODO: more deeply nested variable patterns, like + // case (a, b) => 1 ; case (c, d) => 2 + // However this is a pain (at least the way I'm going about it) + // and I have to think these detailed errors are primarily useful + // for beginners, not people writing nested pattern matches. + def checkMatchVariablePatterns(m: Match) { + // A string describing the first variable pattern + var vpat: String = null + // Using an iterator so we can recognize the last case + val it = m.cases.iterator + + def addendum(pat: Tree) = { + matchingSymbolInScope(pat) match { + case NoSymbol => "" + case sym => + val desc = if (sym.isParameter) s"parameter ${sym.nameString} of" else sym + " in" + s"\nIf you intended to match against $desc ${sym.owner}, you must use backticks, like: case `${sym.nameString}` =>" + } + } + + while (it.hasNext) { + val cdef = it.next + // If a default case has been seen, then every succeeding case is unreachable. + if (vpat != null) + context.unit./*error*/warning(cdef.body.pos, "unreachable code due to " + vpat + addendum(cdef.pat)) + // If this is a default case and more cases follow, warn about this one so + // we have a reason to mention its pattern variable name and any corresponding + // symbol in scope. Errors will follow from the remaining cases, at least + // once we make the above warning an error. + else if (it.hasNext && (treeInfo isDefaultCase cdef)) { + val vpatName = cdef.pat match { + case Bind(name, _) => s" '$name'" + case _ => "" + } + vpat = s"variable pattern$vpatName on line ${cdef.pat.pos.line}" + context.unit.warning(cdef.pos, s"patterns after a variable pattern cannot match (SLS 8.1.1)" + addendum(cdef.pat)) + } + } + } + /** Implement a pattern match by turning its cases (including the implicit failure case) * into the corresponding (monadic) extractors, and combining them with the `orElse` combinator. * @@ -210,6 +273,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL */ def translateMatch(match_ : Match): Tree = { val Match(selector, cases) = match_ + checkMatchVariablePatterns(match_) // we don't transform after uncurry // (that would require more sophistication when generating trees, @@ -3358,16 +3422,12 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL case Some(cds) => cds } - val allReachable = - if (unchecked) true - else { - val unreachables = unreachableCase(caseDefsWithGuards) - unreachables foreach {cd => reportUnreachable(cd.body.pos)} - // a switch with duplicate cases yields a verify error, - // and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch - // (even though the verify error would disappear, the behaviour would change) - unreachables.isEmpty - } + val allReachable = unchecked || { + // a switch with duplicate cases yields a verify error, + // and a switch with duplicate cases and guards cannot soundly be rewritten to an unguarded switch + // (even though the verify error would disappear, the behaviour would change) + unreachableCase(caseDefsWithGuards) map (cd => reportUnreachable(cd.body.pos)) isEmpty + } if (!allReachable) Nil else if (noGuards(caseDefsWithGuards)) { diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index 1b4c1b2877..92a6156e54 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -236,7 +236,7 @@ abstract class TreeInfo { case _ => tree } - + /** Is tree a self or super constructor call? */ def isSelfOrSuperConstrCall(tree: Tree) = { // stripNamedApply for SI-3584: adaptToImplicitMethod in Typers creates a special context diff --git a/test/files/neg/macro-invalidsig-context-bounds.check b/test/files/neg/macro-invalidsig-context-bounds.check index 894eabc442..6c9482e537 100644 --- a/test/files/neg/macro-invalidsig-context-bounds.check +++ b/test/files/neg/macro-invalidsig-context-bounds.check @@ -1,4 +1,4 @@ -Impls_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences
- def foo[U: c.AbsTypeTag: Numeric](c: Ctx) = {
- ^
-one error found
+Impls_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences + def foo[U: c.AbsTypeTag: Numeric](c: Ctx) = { + ^ +one error found diff --git a/test/files/neg/macro-invalidsig-implicit-params.check b/test/files/neg/macro-invalidsig-implicit-params.check index 029b8a4634..98b3167b7a 100644 --- a/test/files/neg/macro-invalidsig-implicit-params.check +++ b/test/files/neg/macro-invalidsig-implicit-params.check @@ -1,4 +1,4 @@ -Impls_Macros_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences
- def foo_targs[T, U: c.AbsTypeTag](c: Ctx)(implicit x: c.Expr[Int]) = {
- ^
-one error found
+Impls_Macros_1.scala:5: error: macro implementations cannot have implicit parameters other than AbsTypeTag evidences + def foo_targs[T, U: c.AbsTypeTag](c: Ctx)(implicit x: c.Expr[Int]) = { + ^ +one error found diff --git a/test/files/neg/newpat_unreachable.check b/test/files/neg/newpat_unreachable.check new file mode 100644 index 0000000000..08453cac19 --- /dev/null +++ b/test/files/neg/newpat_unreachable.check @@ -0,0 +1,27 @@ +newpat_unreachable.scala:6: error: patterns after a variable pattern cannot match (SLS 8.1.1) +If you intended to match against parameter b of method contrivedExample, you must use backticks, like: case `b` => + case b => println("matched b") + ^ +newpat_unreachable.scala:7: error: unreachable code due to variable pattern 'b' on line 6 +If you intended to match against parameter c of method contrivedExample, you must use backticks, like: case `c` => + case c => println("matched c") + ^ +newpat_unreachable.scala:8: error: unreachable code due to variable pattern 'b' on line 6 +If you intended to match against value d in class A, you must use backticks, like: case `d` => + case d => println("matched d") + ^ +newpat_unreachable.scala:9: error: unreachable code due to variable pattern 'b' on line 6 + case _ => println("matched neither") + ^ +newpat_unreachable.scala:22: error: patterns after a variable pattern cannot match (SLS 8.1.1) +If you intended to match against parameter b of method g, you must use backticks, like: case `b` => + case b => 1 + ^ +newpat_unreachable.scala:23: error: unreachable code due to variable pattern 'b' on line 22 +If you intended to match against parameter c of method h, you must use backticks, like: case `c` => + case c => 2 + ^ +newpat_unreachable.scala:24: error: unreachable code due to variable pattern 'b' on line 22 + case _ => 3 + ^ +7 errors found diff --git a/test/files/neg/newpat_unreachable.flags b/test/files/neg/newpat_unreachable.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/newpat_unreachable.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/newpat_unreachable.scala b/test/files/neg/newpat_unreachable.scala new file mode 100644 index 0000000000..c9cc85cec6 --- /dev/null +++ b/test/files/neg/newpat_unreachable.scala @@ -0,0 +1,29 @@ +object Test { + class A { + val d = 55 + + def contrivedExample[A, B, C](a: A, b: B, c: C): Unit = a match { + case b => println("matched b") + case c => println("matched c") + case d => println("matched d") + case _ => println("matched neither") + } + + def correctExample[A, B, C](a: A, b: B, c: C): Unit = a match { + case `b` => println("matched b") + case `c` => println("matched c") + case `d` => println("matched d") + case _ => println("matched neither") + } + + def f[A](a: A) = { + def g[B](b: B) = { + def h[C](c: C) = a match { + case b => 1 + case c => 2 + case _ => 3 + } + } + } + } +} diff --git a/test/files/neg/pat_unreachable.check b/test/files/neg/pat_unreachable.check index 4e1463d591..c5706b7fad 100644 --- a/test/files/neg/pat_unreachable.check +++ b/test/files/neg/pat_unreachable.check @@ -4,4 +4,10 @@ pat_unreachable.scala:5: error: unreachable code pat_unreachable.scala:9: error: unreachable code case Seq(x, y) => List(x, y) ^ -two errors found +pat_unreachable.scala:23: error: unreachable code + case c => println("matched c") + ^ +pat_unreachable.scala:24: error: unreachable code + case _ => println("matched neither") + ^ +four errors found diff --git a/test/files/neg/pat_unreachable.scala b/test/files/neg/pat_unreachable.scala index fc0fd41920..1f402e5212 100644 --- a/test/files/neg/pat_unreachable.scala +++ b/test/files/neg/pat_unreachable.scala @@ -8,7 +8,7 @@ object Test extends App { case Seq(x, y, _*) => x::y::Nil case Seq(x, y) => List(x, y) } - + def not_unreachable(xs:Seq[Char]) = xs match { case Seq(x, y, _*) => x::y::Nil case Seq(x) => List(x) @@ -17,4 +17,10 @@ object Test extends App { case Seq(x, y) => x::y::Nil case Seq(x, y, z, _*) => List(x,y) } + + def contrivedExample[A, B, C](a: A, b: B, c: C): Unit = a match { + case b => println("matched b") + case c => println("matched c") + case _ => println("matched neither") + } } diff --git a/test/files/neg/t6048.check b/test/files/neg/t6048.check index 051f41877e..5bdf2eca88 100644 --- a/test/files/neg/t6048.check +++ b/test/files/neg/t6048.check @@ -4,7 +4,10 @@ t6048.scala:3: error: unreachable code t6048.scala:8: error: unreachable code case _ if false => x // unreachable ^ -t6048.scala:14: error: unreachable code +t6048.scala:13: error: patterns after a variable pattern cannot match (SLS 8.1.1) + case _ => x + ^ +t6048.scala:14: error: unreachable code due to variable pattern on line 13 case 5 if true => x // unreachable ^ -three errors found +four errors found |