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

More conservative optimization for unnecessary outer ref checks

3 files changed, 31 insertions, 23 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.