Merge pull request #5612 from adriaanm/patmat_cleanup

Cleanup in aisle patmat

5 files changed, 38 insertions, 32 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
index 91777b34d5..40fcceb0bf 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/Logic.scala
@@ -691,7 +691,7 @@ trait ScalaLogic extends Interface with Logic with TreeAndTypeAnalysis {
           // if X is mutable.
           freshExistentialSubtype(t.tpe)
         }
-        else trees find (a => a.correspondsStructure(t)(sameValue)) match {
+        else trees find (a => equivalentTree(a, t)) match {
           case Some(orig) =>
             debug.patmat("unique tp for tree: " + ((orig, orig.tpe)))
             orig.tpe
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
index 1331eb6993..c71299b893 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
@@ -85,11 +85,15 @@ trait TreeAndTypeAnalysis extends Debugging {
     tp <:< tpImpliedNormalizedToAny
   }
 
-  // TODO: improve, e.g., for constants
-  def sameValue(a: Tree, b: Tree): Boolean = (a eq b) || ((a, b) match {
-    case (_ : Ident, _ : Ident) => a.symbol eq b.symbol
-    case _                      => false
-  })
+  def equivalentTree(a: Tree, b: Tree): Boolean = (a, b) match {
+    case (Select(qual1, _), Select(qual2, _)) => equivalentTree(qual1, qual2) && a.symbol == b.symbol
+    case (Ident(_), Ident(_)) => a.symbol == b.symbol
+    case (Literal(c1), Literal(c2)) => c1 == c2
+    case (This(_), This(_)) => a.symbol == b.symbol
+    case (Apply(fun1, args1), Apply(fun2, args2)) => equivalentTree(fun1, fun2) && args1.corresponds(args2)(equivalentTree)
+    // Those are the only cases we need to handle in the pattern matcher
+    case _ => false
+  }
 
   trait CheckableTreeAndTypeAnalysis {
     val typer: Typer
@@ -277,7 +281,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
 
       // hashconsing trees (modulo value-equality)
       def unique(t: Tree, tpOverride: Type = NoType): Tree =
-        trees find (a => a.correspondsStructure(t)(sameValue)) match {
+        trees find (a => equivalentTree(a, t)) match {
           case Some(orig) =>
             // debug.patmat("unique: "+ (t eq orig, orig))
             orig
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
index cca8d2dbb8..f827043094 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala
@@ -116,8 +116,8 @@ trait MatchOptimization extends MatchTreeMaking with MatchAnalysis {
               case _ =>
             }
 
-            debug.patmat("sharedPrefix: "+ sharedPrefix)
-            debug.patmat("suffix: "+ sharedPrefix)
+            debug.patmat(s"sharedPrefix: $sharedPrefix")
+            debug.patmat(s"suffix: $suffix")
             // if the shared prefix contains interesting conditions (!= True)
             // and the last of such interesting shared conditions reuses another treemaker's test
             // replace the whole sharedPrefix by a ReusingCondTreeMaker
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala
index a2afb76b0e..bf3bc6b26e 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTranslation.scala
@@ -110,25 +110,29 @@ trait MatchTranslation {
 
       // example check: List[Int] <:< ::[Int]
       private def extractorStep(): TranslationStep = {
-        def paramType = extractor.aligner.wholeType
         import extractor.treeMaker
-        // chain a type-testing extractor before the actual extractor call
-        // it tests the type, checks the outer pointer and casts to the expected type
-        // TODO: the outer check is mandated by the spec for case classes, but we do it for user-defined unapplies as well [SPEC]
-        // (the prefix of the argument passed to the unapply must equal the prefix of the type of the binder)
-        lazy val typeTest = TypeTestTreeMaker(binder, binder, paramType, paramType)(pos, extractorArgTypeTest = true)
-        // check whether typetest implies binder is not null,
-        // even though the eventual null check will be on typeTest.nextBinder
-        // it'll be equal to binder casted to paramType anyway (and the type test is on binder)
-        def extraction: TreeMaker = treeMaker(typeTest.nextBinder, typeTest impliesBinderNonNull binder, pos)
 
         // paramType = the type expected by the unapply
         // TODO: paramType may contain unbound type params (run/t2800, run/t3530)
-        val makers = (
+        val makers = {
+          val paramType = extractor.aligner.wholeType
           // Statically conforms to paramType
           if (this ensureConformsTo paramType) treeMaker(binder, false, pos) :: Nil
-          else typeTest :: extraction :: Nil
-        )
+          else {
+            // chain a type-testing extractor before the actual extractor call
+            // it tests the type, checks the outer pointer and casts to the expected type
+            // TODO: the outer check is mandated by the spec for case classes, but we do it for user-defined unapplies as well [SPEC]
+            // (the prefix of the argument passed to the unapply must equal the prefix of the type of the binder)
+            val typeTest = TypeTestTreeMaker(binder, binder, paramType, paramType)(pos, extractorArgTypeTest = true)
+            val binderKnownNonNull = typeTest impliesBinderNonNull binder
+
+            // check whether typetest implies binder is not null,
+            // even though the eventual null check will be on typeTest.nextBinder
+            // it'll be equal to binder casted to paramType anyway (and the type test is on binder)
+            typeTest :: treeMaker(typeTest.nextBinder, binderKnownNonNull, pos) :: Nil
+          }
+        }
+
         step(makers: _*)(extractor.subBoundTrees: _*)
       }
 
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
index e0fcc05de2..3ace61411f 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
@@ -337,17 +337,15 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
         def eqTest(pat: Tree, testedBinder: Symbol)          = REF(testedBinder) OBJ_EQ pat
 
         def outerTest(testedBinder: Symbol, expectedTp: Type): Tree = {
-          val expectedOuter = expectedTp.prefix match {
-            case ThisType(clazz) => This(clazz)
-            case NoType          => mkTRUE // fallback for SI-6183
-            case pre             => REF(pre.prefix, pre.termSymbol)
+          val expectedPrefix = expectedTp.prefix
+          if (expectedPrefix eq NoType) mkTRUE // fallback for SI-6183
+          else {
+            // ExplicitOuter replaces `Select(q, outerSym) OBJ_EQ expectedPrefix` by `Select(q, outerAccessor(outerSym.owner)) OBJ_EQ expectedPrefix`
+            // if there's an outer accessor, otherwise the condition becomes `true` -- TODO: can we improve needsOuterTest so there's always an outerAccessor?
+            val outerFor = expectedTp.typeSymbol
+            val outerMarker = outerFor.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedPrefix
+            Select(codegen._asInstanceOf(testedBinder, expectedTp), outerMarker) OBJ_EQ gen.mkAttributedQualifier(expectedPrefix)
           }
-
-          // ExplicitOuter replaces `Select(q, outerSym) OBJ_EQ expectedPrefix` by `Select(q, outerAccessor(outerSym.owner)) OBJ_EQ expectedPrefix`
-          // if there's an outer accessor, otherwise the condition becomes `true` -- TODO: can we improve needsOuterTest so there's always an outerAccessor?
-          val outer = expectedTp.typeSymbol.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedTp.prefix
-
-          (Select(codegen._asInstanceOf(testedBinder, expectedTp), outer)) OBJ_EQ expectedOuter
         }
       }