From 4a16b044baf8377e04624207202e83c78a0a49cf Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Fri, 21 Mar 2014 11:54:02 +0100 Subject: SI-8430 Less non-determinism in patmat exhautiveness warnings Another mole whacked on the head by using `LinkedHashMap`. Caution: `LinkedHashMap` doesn't preserve its runtime type if you map through the generic interface. I've noted this gotcha as SI-8434. I've structured this patch to enforce that concrete collection with types, which is a good idea anyway. My method to track this down was to place breakpoints in `Hash{Map,Set}`.{foreach,iterator}` to see where that was used from within pattern match translation. This approach was drastically faster than my previous rounds of whack-a-mole. The counter-examples are still a bit off; I'm going to merge that aspect of this ticket with SI-7746, in which we've pinpointed the culpable part of the implementation, but haven't had success in fixing the bug. --- .../scala/tools/nsc/transform/patmat/Logic.scala | 12 ++++---- test/files/neg/t8430.check | 27 ++++++++++++++++++ test/files/neg/t8430.flags | 1 + test/files/neg/t8430.scala | 32 ++++++++++++++++++++++ 4 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 test/files/neg/t8430.check create mode 100644 test/files/neg/t8430.flags create mode 100644 test/files/neg/t8430.scala diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala index e0bc478fad..ffd3a30d3a 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala @@ -86,7 +86,7 @@ trait Logic extends Debugging { def mayBeNull: Boolean // compute the domain and return it (call registerNull first!) - def domainSyms: Option[Set[Sym]] + def domainSyms: Option[mutable.LinkedHashSet[Sym]] // the symbol for this variable being equal to its statically known type // (only available if registerEquality has been called for that type before) @@ -197,7 +197,7 @@ trait Logic extends Debugging { def removeVarEq(props: List[Prop], modelNull: Boolean = false): (Formula, List[Formula]) = { val start = if (Statistics.canEnable) Statistics.startTimer(patmatAnaVarEq) else null - val vars = new scala.collection.mutable.HashSet[Var] + val vars = mutable.LinkedHashSet[Var]() object gatherEqualities extends PropTraverser { override def apply(p: Prop) = p match { @@ -334,9 +334,9 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis { // we enumerate the subtypes of the full type, as that allows us to filter out more types statically, // once we go to run-time checks (on Const's), convert them to checkable types // TODO: there seems to be bug for singleton domains (variable does not show up in model) - lazy val domain: Option[Set[Const]] = { - val subConsts = enumerateSubtypes(staticTp).map{ tps => - tps.toSet[Type].map{ tp => + lazy val domain: Option[mutable.LinkedHashSet[Const]] = { + val subConsts: Option[mutable.LinkedHashSet[Const]] = enumerateSubtypes(staticTp).map { tps => + mutable.LinkedHashSet(tps: _*).map{ tp => val domainC = TypeConst(tp) registerEquality(domainC) domainC @@ -479,7 +479,7 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis { } // accessing after calling registerNull will result in inconsistencies - lazy val domainSyms: Option[Set[Sym]] = domain map { _ map symForEqualsTo } + lazy val domainSyms: Option[collection.mutable.LinkedHashSet[Sym]] = domain map { _ map symForEqualsTo } lazy val symForStaticTp: Option[Sym] = symForEqualsTo.get(TypeConst(staticTpCheckable)) diff --git a/test/files/neg/t8430.check b/test/files/neg/t8430.check new file mode 100644 index 0000000000..7c6a73ce53 --- /dev/null +++ b/test/files/neg/t8430.check @@ -0,0 +1,27 @@ +t8430.scala:15: warning: match may not be exhaustive. +It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + ^ +t8430.scala:16: warning: match may not be exhaustive. +It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + ^ +t8430.scala:17: warning: match may not be exhaustive. +It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + ^ +t8430.scala:18: warning: match may not be exhaustive. +It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + ^ +t8430.scala:19: warning: match may not be exhaustive. +It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + ^ +t8430.scala:20: warning: match may not be exhaustive. +It would fail on the following inputs: ??, LetC, LetF, LetL(IntLit), LetP + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + ^ +error: No warnings can be incurred under -Xfatal-warnings. +6 warnings found +one error found diff --git a/test/files/neg/t8430.flags b/test/files/neg/t8430.flags new file mode 100644 index 0000000000..85d8eb2ba2 --- /dev/null +++ b/test/files/neg/t8430.flags @@ -0,0 +1 @@ +-Xfatal-warnings diff --git a/test/files/neg/t8430.scala b/test/files/neg/t8430.scala new file mode 100644 index 0000000000..4166b08a0a --- /dev/null +++ b/test/files/neg/t8430.scala @@ -0,0 +1,32 @@ +sealed trait CL3Literal +case object IntLit extends CL3Literal +case object CharLit extends CL3Literal +case object BooleanLit extends CL3Literal +case object UnitLit extends CL3Literal + + +sealed trait Tree +case class LetL(value: CL3Literal) extends Tree +case object LetP extends Tree +case object LetC extends Tree +case object LetF extends Tree + +object Test { + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + (tree: Tree) => tree match {case LetL(CharLit) => ??? } + // After the first patch for SI-8430, we achieve stability: all of + // these get the same warning: + // + // ??, LetC, LetF, LetL(IntLit), LetP + // + // Before, it was non-deterministic. + // + // However, we our list of counter examples is itself non-exhaustive. + // We need to rework counter example generation to fix that. + // + // That work is the subject of SI-7746 +} -- cgit v1.2.3