Improved outer ref checking in pattern matches:

The old algorithm omitted necessary outer ref checks in some places.
This new one is more conservative. It only omits outer ref checks when
the expected type and the scrutinee type match up, or when the expected
type is defined in a static location. For this specific purpose the top
level of a method or other code block (which is not a trait or class
definition) is also considered static because it does not have a prefix.

This change comes with a spec update to clarify the prefix rule for type
patterns. The new wording makes it clear that the presence of a prefix
is to be interpreted in a *semantic* way, i.e. the existence of a prefix
determines the necessity for an outer ref check, no matter if the prefix
is actually spelled out *syntactically*. Note that the old outer ref
check implementation did not use the alternative interpretation of
requiring prefixes to be given syntactically. It never created an outer
ref check for a local class `C`, no matter if the pattern was `_: C`
or `_: this.C`, thus violating both interpretations of the spec.

There is now explicit support for unchecked matches (like
`case _: (T @unchecked) =>`) to suppress warnings for unchecked outer
refs. `@unchecked` worked before and was used for this purpose in
`neg/t7721` but never actually existed as a feature. It was a result of
a bug that prevented an outer ref check from being generated in the
first place if *any* annotation was used on an expected type in a type
pattern. This new version will still generate the outer ref check if an
outer ref is available but suppress the warning otherwise. Other
annotations on type patterns are ignored.

New tests are in `neg/outer-ref-checks`. The expected results of tests
`neg/t7171` and `neg/t7171b` have changed because the compiler now
tries to generate additional outer ref checks that were not present
before (which was a bug).

12 files changed, 204 insertions, 68 deletions

diff --git a/spec/08-pattern-matching.md b/spec/08-pattern-matching.md
index d496388a91..7e48947639 100644
--- a/spec/08-pattern-matching.md
+++ b/spec/08-pattern-matching.md
@@ -328,10 +328,12 @@ A type pattern $T$ is of one of the following  forms:
 
 * A reference to a class $C$, $p.C$, or `$T$#$C$`.  This
   type pattern matches any non-null instance of the given class.
-  Note that the prefix of the class, if it is given, is relevant for determining
+  Note that the prefix of the class, if it exists, is relevant for determining
   class instances. For instance, the pattern $p.C$ matches only
   instances of classes $C$ which were created with the path $p$ as
-  prefix.
+  prefix. This also applies to prefixes which are not given syntactically.
+  For example, if $C$ refers to a class defined in the nearest enclosing
+  class and is thus equivalent to $this.C$, it is considered to have a prefix.
 
   The bottom types `scala.Nothing` and `scala.Null` cannot
   be used as type patterns, because they would match nothing in any case.
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 {
diff --git a/test/files/neg/outer-ref-checks.check b/test/files/neg/outer-ref-checks.check
new file mode 100644
index 0000000000..bba7118d79
--- /dev/null
+++ b/test/files/neg/outer-ref-checks.check
@@ -0,0 +1,24 @@
+outer-ref-checks.scala:5: warning: The outer reference in this type test cannot be checked at run time.
+  final case class Inner(val s: String) // unchecked warning
+                   ^
+outer-ref-checks.scala:8: warning: The outer reference in this type test cannot be checked at run time.
+    case Inner(s) => // unchecked warning
+              ^
+outer-ref-checks.scala:18: warning: The outer reference in this type test cannot be checked at run time.
+    case Inner(s) => // unchecked warning
+              ^
+outer-ref-checks.scala:19: warning: The outer reference in this type test cannot be checked at run time.
+    case O.Inner(s) => // unchecked warning
+                ^
+outer-ref-checks.scala:41: warning: The outer reference in this type test cannot be checked at run time.
+    case Inner(s) => // unchecked warning
+              ^
+outer-ref-checks.scala:46: warning: The outer reference in this type test cannot be checked at run time.
+    case _: Inner => // unchecked warning
+          ^
+outer-ref-checks.scala:56: warning: The outer reference in this type test cannot be checked at run time.
+    case _: (Inner @uncheckedVariance) => // unchecked warning
+          ^
+error: No warnings can be incurred under -Xfatal-warnings.
+7 warnings found
+one error found
diff --git a/test/files/neg/outer-ref-checks.flags b/test/files/neg/outer-ref-checks.flags
new file mode 100644
index 0000000000..464cc20ea6
--- /dev/null
+++ b/test/files/neg/outer-ref-checks.flags
@@ -0,0 +1 @@
+-Xfatal-warnings -unchecked
\ No newline at end of file
diff --git a/test/files/neg/outer-ref-checks.scala b/test/files/neg/outer-ref-checks.scala
new file mode 100644
index 0000000000..35983fe92b
--- /dev/null
+++ b/test/files/neg/outer-ref-checks.scala
@@ -0,0 +1,106 @@
+import scala.annotation.unchecked.uncheckedVariance
+
+class Outer {
+  // A final class gets no outer ref, so we expect to see warnings where an outer ref check should be performed
+  final case class Inner(val s: String) // unchecked warning
+
+  def belongs(a: Any): Unit = a match {
+    case Inner(s) => // unchecked warning
+    case _ =>
+  }
+
+  def belongsStaticSameOuter(a: Inner): Unit = a match {
+    case Inner(s) => // no need for outer check
+    // match is exhaustive, no default case needed
+  }
+
+  def belongsOtherOuter(a: Outer#Inner): Unit = a match {
+    case Inner(s) => // unchecked warning
+    case O.Inner(s) => // unchecked warning
+    case _ =>
+  }
+}
+
+object O extends Outer {
+  def belongsStaticSameOuter2(a: Inner): Unit = a match {
+    case Inner(s) => // no need for outer check
+    // match is exhaustive, no default case needed
+  }
+
+  def belongsStaticSameOuter3(a: Inner): Unit = a match {
+    case _: Inner => // no need for outer check
+    // match is exhaustive, no default case needed
+  }
+
+  def belongsStaticSameOuter4(a: Inner): Unit = a match {
+    case _: (Inner @uncheckedVariance) => // no need for outer check
+    // match is exhaustive, no default case needed
+  }
+
+  def belongsOtherOuter2(a: Outer#Inner): Unit = a match {
+    case Inner(s) => // unchecked warning
+    case _ =>
+  }
+
+  def belongsOtherOuter3(a: Outer#Inner): Unit = a match {
+    case _: Inner => // unchecked warning
+    case _ =>
+  }
+
+  def belongsOtherOuter4(a: Outer#Inner): Unit = a match {
+    case _: (Inner @unchecked) => // warning supressed
+    case _ =>
+  }
+
+  def belongsOtherOuter5(a: Outer#Inner): Unit = a match {
+    case _: (Inner @uncheckedVariance) => // unchecked warning
+    case _ =>
+  }
+
+  def nested: Unit = {
+    final case class I(s: String)
+
+    def check1(a: Any): Unit = a match {
+      case I(s) => // no need for outer check
+      case _ =>
+    }
+
+    def check2(a: I): Unit = a match {
+      case I(s) => // no need for outer check
+      // match is exhaustive, no default case needed
+    }
+  }
+}
+
+class O2 {
+  def nested: Unit = {
+    final case class I(s: String)
+
+    def check1(a: Any): Unit = a match {
+      case I(s) => // no need for outer check (is this correct?)
+      case _ =>
+    }
+
+    def check2(a: I): Unit = a match {
+      case I(s) => // no need for outer check (is this correct?)
+      // match is exhaustive, no default case needed
+    }
+  }
+}
+
+package p {
+  object T {
+    case class C(x: Int)
+  }
+}
+
+object U {
+  val T = p.T
+}
+
+class Test {
+  def m(a: Any) = a match {
+    case U.T.C(1) => 1  // used to warn
+    case _ => 1
+  }
+}
diff --git a/test/files/neg/t7171.check b/test/files/neg/t7171.check
index ecd768afda..2de9151483 100644
--- a/test/files/neg/t7171.check
+++ b/test/files/neg/t7171.check
@@ -1,6 +1,9 @@
 t7171.scala:2: warning: The outer reference in this type test cannot be checked at run time.
   final case class A()
                    ^
+t7171.scala:9: warning: The outer reference in this type test cannot be checked at run time.
+    case _: A => true; case _ => false
+          ^
 error: No warnings can be incurred under -Xfatal-warnings.
-one warning found
+two warnings found
 one error found
diff --git a/test/files/neg/t7171b.check b/test/files/neg/t7171b.check
index bf695afea7..6b05b6fa63 100644
--- a/test/files/neg/t7171b.check
+++ b/test/files/neg/t7171b.check
@@ -1,6 +1,12 @@
 t7171b.scala:2: warning: The outer reference in this type test cannot be checked at run time.
   final case class A()
                    ^
+t7171b.scala:8: warning: The outer reference in this type test cannot be checked at run time.
+    case _: A => true; case _ => false
+          ^
+t7171b.scala:13: warning: The outer reference in this type test cannot be checked at run time.
+    case _: A => true; case _ => false
+          ^
 error: No warnings can be incurred under -Xfatal-warnings.
-one warning found
+three warnings found
 one error found
diff --git a/test/files/run/t7171.check b/test/files/run/t7171.check
index d826f6cb94..5454142882 100644
--- a/test/files/run/t7171.check
+++ b/test/files/run/t7171.check
@@ -1,3 +1,6 @@
 t7171.scala:2: warning: The outer reference in this type test cannot be checked at run time.
   final case class A()
                    ^
+t7171.scala:9: warning: The outer reference in this type test cannot be checked at run time.
+    case _: A => true; case _ => false
+          ^