Merge pull request #4974 from szeiger/wip/patmat-outertest

More conservative optimization for unnecessary outer ref checks

5 files changed, 55 insertions, 64 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index 1a0b4cdec0..4abcc774da 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -486,7 +486,8 @@ abstract class ExplicitOuter extends InfoTransform
               // since we can't fix SI-4440 properly (we must drop the outer accessors of final classes when there's no immediate reference to them in sight)
               // at least don't crash... this duplicates maybeOmittable from constructors
               (acc.owner.isEffectivelyFinal && !acc.isOverridingSymbol)) {
-            currentRun.reporting.uncheckedWarning(tree.pos, "The outer reference in this type test cannot be checked at run time.")
+            if (!base.tpe.hasAnnotation(UncheckedClass))
+              currentRun.reporting.uncheckedWarning(tree.pos, "The outer reference in this type test cannot be checked at run time.")
             transform(TRUE) // urgh... drop condition if there's no accessor (or if it may disappear after constructors)
           } else {
             // println("(base, acc)= "+(base, acc))
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
index ab3b25e76b..2b16995f0b 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
@@ -351,7 +351,7 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
               object condStrategy extends TypeTestTreeMaker.TypeTestCondStrategy {
                 type Result                                           = Prop
                 def and(a: Result, b: Result)                         = And(a, b)
-                def outerTest(testedBinder: Symbol, expectedTp: Type) = True // TODO OuterEqProp(testedBinder, expectedType)
+                def withOuterTest(testedBinder: Symbol, expectedTp: Type) = True // TODO OuterEqProp(testedBinder, expectedType)
                 def typeTest(b: Symbol, pt: Type) = { // a type test implies the tested path is non-null (null.isInstanceOf[T] is false for all T)
                   val p = binderToUniqueTree(b);                        And(uniqueNonNullProp(p), uniqueTypeProp(p, uniqueTp(pt)))
                 }
diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
index c6e7f8fcda..8c59ced28f 100644
--- a/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
+++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchTreeMaking.scala
@@ -316,7 +316,7 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
       trait TypeTestCondStrategy {
         type Result
 
-        def outerTest(testedBinder: Symbol, expectedTp: Type): Result
+        def withOuterTest(orig: Result)(testedBinder: Symbol, expectedTp: Type): Result = orig
         // TODO: can probably always widen
         def typeTest(testedBinder: Symbol, expectedTp: Type): Result
         def nonNullTest(testedBinder: Symbol): Result
@@ -336,18 +336,34 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
         def equalsTest(pat: Tree, testedBinder: Symbol)      = codegen._equals(pat, testedBinder)
         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)
+        override def withOuterTest(orig: Tree)(testedBinder: Symbol, expectedTp: Type): Tree = {
+          val expectedPrefix = expectedTp.prefix
+          val testedPrefix = testedBinder.info.prefix
+
+          // Check if a type is defined in a static location. Unlike `tp.isStatic` before `flatten`,
+          // this also includes methods and (possibly nested) objects inside of methods.
+          def definedInStaticLocation(tp: Type): Boolean = {
+            def isStatic(tp: Type): Boolean =
+              if (tp == NoType || tp.typeSymbol.isPackageClass || tp == NoPrefix) true
+              else if (tp.typeSymbol.isModuleClass) isStatic(tp.prefix)
+              else false
+            tp.typeSymbol.owner == tp.prefix.typeSymbol && isStatic(tp.prefix)
           }
 
-          // 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
+          if ((expectedPrefix eq NoPrefix)
+            || definedInStaticLocation(expectedTp)
+            || testedPrefix =:= expectedPrefix) orig
+          else gen.mkAttributedQualifierIfPossible(expectedPrefix) match {
+            case None => orig
+            case Some(expectedOuterRef) =>
+              // 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: centralize logic whether there's an outer accessor and use here?
+              val synthOuterGetter = expectedTp.typeSymbol.newMethod(vpmName.outer, newFlags = SYNTHETIC | ARTIFACT) setInfo expectedPrefix
+              val outerTest = (Select(codegen._asInstanceOf(testedBinder, expectedTp), synthOuterGetter)) OBJ_EQ expectedOuterRef
+              and(orig, outerTest)
+          }
         }
       }
 
@@ -356,7 +372,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
 
         def typeTest(testedBinder: Symbol, expectedTp: Type): Result  = true
 
-        def outerTest(testedBinder: Symbol, expectedTp: Type): Result = false
         def nonNullTest(testedBinder: Symbol): Result                 = false
         def equalsTest(pat: Tree, testedBinder: Symbol): Result       = false
         def eqTest(pat: Tree, testedBinder: Symbol): Result           = false
@@ -368,7 +383,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
         type Result = Boolean
 
         def typeTest(testedBinder: Symbol, expectedTp: Type): Result  = testedBinder eq binder
-        def outerTest(testedBinder: Symbol, expectedTp: Type): Result = false
         def nonNullTest(testedBinder: Symbol): Result                 = testedBinder eq binder
         def equalsTest(pat: Tree, testedBinder: Symbol): Result       = false // could in principle analyse pat and see if it's statically known to be non-null
         def eqTest(pat: Tree, testedBinder: Symbol): Result           = false // could in principle analyse pat and see if it's statically known to be non-null
@@ -405,12 +419,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
       import TypeTestTreeMaker._
       debug.patmat("TTTM"+((prevBinder, extractorArgTypeTest, testedBinder, expectedTp, nextBinderTp)))
 
-      lazy val outerTestNeeded = (
-           (expectedTp.prefix ne NoPrefix)
-        && !expectedTp.prefix.typeSymbol.isPackageClass
-        && needsOuterTest(expectedTp, testedBinder.info, matchOwner)
-      )
-
       // the logic to generate the run-time test that follows from the fact that
       // a `prevBinder` is expected to have type `expectedTp`
       // the actual tree-generation logic is factored out, since the analyses generate Cond(ition)s rather than Trees
@@ -429,12 +437,11 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
         def isExpectedPrimitiveType = isAsExpected && isPrimitiveValueType(expectedTp)
         def isExpectedReferenceType = isAsExpected && (expectedTp <:< AnyRefTpe)
         def mkNullTest              = nonNullTest(testedBinder)
-        def mkOuterTest             = outerTest(testedBinder, expectedTp)
         def mkTypeTest              = typeTest(testedBinder, expectedWide)
 
         def mkEqualsTest(lhs: Tree): cs.Result      = equalsTest(lhs, testedBinder)
         def mkEqTest(lhs: Tree): cs.Result          = eqTest(lhs, testedBinder)
-        def addOuterTest(res: cs.Result): cs.Result = if (outerTestNeeded) and(res, mkOuterTest) else res
+        def addOuterTest(res: cs.Result): cs.Result = withOuterTest(res)(testedBinder, expectedTp)
 
         // If we conform to expected primitive type:
         //   it cannot be null and cannot have an outer pointer. No further checking.
diff --git a/src/reflect/scala/reflect/internal/TreeGen.scala b/src/reflect/scala/reflect/internal/TreeGen.scala
index 2933aa3892..ec6426558c 100644
--- a/src/reflect/scala/reflect/internal/TreeGen.scala
+++ b/src/reflect/scala/reflect/internal/TreeGen.scala
@@ -117,6 +117,30 @@ abstract class TreeGen {
     case _                       => qual
   }
 
+
+
+  //          val selType = testedBinder.info
+  //
+  //          // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest`
+  //          // generates an outer test based on `patType.prefix` with automatically dealises.
+  //          // Prefixes can have all kinds of shapes SI-9110
+  //          val patPre = expectedTp.dealiasWiden.prefix
+  //          val selPre = selType.dealiasWiden.prefix
+  //
+  //          // Optimization: which prefixes can we disqualify from the need for an outer reference check?
+  //          //   - classes in static owners do not get outer pointers
+  //          //   - if the prefixes are statically known to be equal, the type system ensures an outer test is redundant
+  //          !((patPre eq NoPrefix) || (selPre eq NoPrefix)
+  //            || patPre.typeSymbol.isPackageClass
+  //            || selPre =:= patPre)
+
+  def mkAttributedQualifierIfPossible(prefix: Type): Option[Tree] = prefix match {
+    case NoType | NoPrefix | ErrorType => None
+    case TypeRef(_, sym, _) if sym.isModule || sym.isClass || sym.isType => None
+    case pre => Some(mkAttributedQualifier(prefix))
+  }
+
+
   /** Builds a reference to given symbol with given stable prefix. */
   def mkAttributedRef(pre: Type, sym: Symbol): RefTree = {
     val qual = mkAttributedQualifier(pre)
diff --git a/src/reflect/scala/reflect/internal/Types.scala b/src/reflect/scala/reflect/internal/Types.scala
index 685c9f7476..24709cf379 100644
--- a/src/reflect/scala/reflect/internal/Types.scala
+++ b/src/reflect/scala/reflect/internal/Types.scala
@@ -3946,47 +3946,6 @@ trait Types
     check(tp1, tp2) && check(tp2, tp1)
   }
 
-  /** Does a pattern of type `patType` need an outer test when executed against
-   *  selector type `selType` in context defined by `currentOwner`?
-   */
-  def needsOuterTest(patType: Type, selType: Type, currentOwner: Symbol) = {
-    def createDummyClone(pre: Type): Type = {
-      val dummy = currentOwner.enclClass.newValue(nme.ANYname).setInfo(pre.widen)
-      singleType(ThisType(currentOwner.enclClass), dummy)
-    }
-    def maybeCreateDummyClone(pre: Type, sym: Symbol): Type = pre match {
-      case SingleType(pre1, sym1) =>
-        if (sym1.isModule && sym1.isStatic) {
-          if (sym.owner == sym1 || sym.isJavaDefined || sym.owner.sourceModule.isStaticModule) NoType
-          else pre
-        } else if (sym1.isModule && sym.owner == sym1.moduleClass) {
-          val pre2 = maybeCreateDummyClone(pre1, sym1)
-          if (pre2 eq NoType) pre2
-          else singleType(pre2, sym1)
-        } else {
-          createDummyClone(pre)
-        }
-      case ThisType(clazz) =>
-        if (clazz.isModuleClass)
-          maybeCreateDummyClone(clazz.typeOfThis, sym)
-        else if (sym.owner == clazz && (sym.hasFlag(PRIVATE) || sym.privateWithin == clazz))
-          NoType
-        else
-          createDummyClone(pre)
-      case _ =>
-        NoType
-    }
-    // See the test for SI-7214 for motivation for dealias. Later `treeCondStrategy#outerTest`
-    // generates an outer test based on `patType.prefix` with automatically dealises.
-    patType.dealias match {
-      case TypeRef(pre, sym, args) =>
-        val pre1 = maybeCreateDummyClone(pre, sym)
-        (pre1 ne NoType) && isPopulated(copyTypeRef(patType, pre1, sym, args), selType)
-      case _ =>
-        false
-    }
-  }
-
   def normalizePlus(tp: Type): Type = {
     if (isRawType(tp)) rawToExistential(tp)
     else tp.normalize match {