SI-6815 untangle isStable and hasVolatileType

`Symbol::isStable` is now independent of `Symbol::hasVolatileType`,
so that we can allow stable identifiers that are volatile in ident patterns.

This split is validated by SI-6815 and the old logic in RefChecks,
which seems to assume this independence, and thus I don't think ever worked:

```
if (member.isStable && !otherTp.isVolatile) {
  if (memberTp.isVolatile)
    overrideError("has a volatile type; cannot override a member with non-volatile type")
```

Introduces `admitsTypeSelection` and `isStableIdentifierPattern` in treeInfo,
and uses them instead of duplicating that logic all over the place.

Since volatility only matters in the context of type application,
`isStableIdentifierPattern` is used to check patterns (resulting in `==` checks)
and imports.

Author: adriaanm

src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala

@@ -141,7 +141,7 @@ trait ContextErrors {
         }
         issueNormalTypeError(tree,
           "stable identifier required, but "+tree+" found." + (
-          if (isStableExceptVolatile(tree)) addendum else ""))
+          if (treeInfo.hasVolatileType(tree)) addendum else ""))
         setError(tree)
       }
 

src/compiler/scala/tools/nsc/typechecker/Namers.scala

@@ -1342,13 +1342,16 @@ trait Namers extends MethodSynthesis {
     private def importSig(imp: Import) = {
       val Import(expr, selectors) = imp
       val expr1 = typer.typedQualifier(expr)
-      typer checkStable expr1
+
       if (expr1.symbol != null && expr1.symbol.isRootPackage)
         RootImportError(imp)
 
       if (expr1.isErrorTyped)
         ErrorType
       else {
+        if (!treeInfo.isStableIdentifierPattern(expr1))
+          typer.TyperErrorGen.UnstableTreeError(expr1)
+
         val newImport = treeCopy.Import(imp, expr1, selectors).asInstanceOf[Import]
         checkSelectors(newImport)
         transformed(imp) = newImport

src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala

@@ -202,7 +202,7 @@ trait NamesDefaults { self: Analyzer =>
           if (module == NoSymbol) None
           else {
             val ref = atPos(pos.focus)(gen.mkAttributedRef(pre, module))
-            if (module.isStable && pre.isStable)    // fixes #4524. the type checker does the same for
+            if (treeInfo.admitsTypeSelection(ref))  // fixes #4524. the type checker does the same for
               ref.setType(singleType(pre, module))  // typedSelect, it calls "stabilize" on the result.
             Some(ref)
           }

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -511,7 +511,10 @@ abstract class RefChecks extends InfoTransform with scala.reflect.internal.trans
             }
 
             if (member.isStable && !otherTp.isVolatile) {
-	            if (memberTp.isVolatile)
+              // (1.4), pt 2 -- member.isStable && memberTp.isVolatile started being possible after SI-6815
+              // (before SI-6815, !symbol.tpe.isVolatile was implied by symbol.isStable)
+              // TODO: allow overriding when @uncheckedStable?
+              if (memberTp.isVolatile)
                 overrideError("has a volatile type; cannot override a member with non-volatile type")
               else memberTp.dealiasWiden.resultType match {
                 case rt: RefinedType if !(rt =:= otherTp) && !(checkedCombinations contains rt.parents) =>

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -587,7 +587,7 @@ trait Typers extends Adaptations with Tags {
       }
 
     /** Post-process an identifier or selection node, performing the following:
-     *  1. Check that non-function pattern expressions are stable
+     *  1. Check that non-function pattern expressions are stable (ignoring volatility concerns -- SI-6815)
      *  2. Check that packages and static modules are not used as values
      *  3. Turn tree type into stable type if possible and required by context.
      *  4. Give getClass calls a more precise type based on the type of the target of the call.
@@ -602,16 +602,18 @@ trait Typers extends Adaptations with Tags {
       if (tree.isErrorTyped) tree
       else if (mode.inPatternNotFunMode && tree.isTerm) { // (1)
         if (sym.isValue) {
-          val tree1 = checkStable(tree)
-          // A module reference in a pattern has type Foo.type, not "object Foo"
-          if (sym.isModuleNotMethod) tree1 setType singleType(pre, sym)
-          else tree1
+          if (tree.isErrorTyped) tree
+          else if (treeInfo.isStableIdentifierPattern(tree)) {
+            // A module reference in a pattern has type Foo.type, not "object Foo"
+            if (sym.isModuleNotMethod) tree setType singleType(pre, sym)
+            else tree
+          } else UnstableTreeError(tree)
         }
         else fail()
       } else if ((mode & (EXPRmode | QUALmode)) == EXPRmode && !sym.isValue && !phase.erasedTypes) { // (2)
         fail()
       } else {
-        if (sym.isStable && pre.isStable && !isByNameParamType(tree.tpe) &&
+        if (treeInfo.admitsTypeSelection(tree) &&
             (isStableContext(tree, mode, pt) || sym.isModuleNotMethod))
           tree.setType(singleType(pre, sym))
         // To fully benefit from special casing the return type of
@@ -4442,6 +4444,7 @@ trait Typers extends Adaptations with Tags {
         }
 
       def normalTypedApply(tree: Tree, fun: Tree, args: List[Tree]) = {
+        // TODO: replace `fun.symbol.isStable` by `treeInfo.isStableIdentifierPattern(fun)`
         val stableApplication = (fun.symbol ne null) && fun.symbol.isMethod && fun.symbol.isStable
         val funpt = if (isPatternMode) pt else WildcardType
         val appStart = if (Statistics.canEnable) Statistics.startTimer(failedApplyNanos) else null
@@ -4744,31 +4747,35 @@ trait Typers extends Adaptations with Tags {
             typedSelect(tree, qual1, nme.CONSTRUCTOR)
           case _ =>
             if (Statistics.canEnable) Statistics.incCounter(typedSelectCount)
-            var qual1 = checkDead(typedQualifier(qual, mode))
-            if (name.isTypeName) qual1 = checkStable(qual1)
+            val qualTyped = checkDead(typedQualifier(qual, mode))
+            val qualStableOrError =
+              if (qualTyped.isErrorTyped || !name.isTypeName || treeInfo.admitsTypeSelection(qualTyped))
+                qualTyped
+              else
+                UnstableTreeError(qualTyped)
 
             val tree1 = // temporarily use `filter` and an alternative for `withFilter`
               if (name == nme.withFilter)
-                silent(_ => typedSelect(tree, qual1, name)) orElse { _ =>
-                    silent(_ => typed1(Select(qual1, nme.filter) setPos tree.pos, mode, pt)) match {
+                silent(_ => typedSelect(tree, qualStableOrError, name)) orElse { _ =>
+                    silent(_ => typed1(Select(qualStableOrError, nme.filter) setPos tree.pos, mode, pt)) match {
                       case SilentResultValue(result2) =>
                         unit.deprecationWarning(
-                          tree.pos, "`withFilter' method does not yet exist on " + qual1.tpe.widen +
+                          tree.pos, "`withFilter' method does not yet exist on " + qualStableOrError.tpe.widen +
                             ", using `filter' method instead")
                         result2
                       case SilentTypeError(err) =>
                         WithFilterError(tree, err)
                     }
                 }
               else
-                typedSelect(tree, qual1, name)
+                typedSelect(tree, qualStableOrError, name)
 
             if (tree.isInstanceOf[PostfixSelect])
               checkFeature(tree.pos, PostfixOpsFeature, name.decode)
             if (tree1.symbol != null && tree1.symbol.isOnlyRefinementMember)
               checkFeature(tree1.pos, ReflectiveCallsFeature, tree1.symbol.toString)
 
-            if (qual1.hasSymbolWhich(_.isRootPackage)) treeCopy.Ident(tree1, name)
+            if (qualStableOrError.hasSymbolWhich(_.isRootPackage)) treeCopy.Ident(tree1, name)
             else tree1
         }
       }
@@ -5161,12 +5168,16 @@ trait Typers extends Adaptations with Tags {
       }
 
       def typedSingletonTypeTree(tree: SingletonTypeTree) = {
-        val ref1 = checkStable(
-          context.withImplicitsDisabled(
+        val refTyped =
+          context.withImplicitsDisabled {
             typed(tree.ref, EXPRmode | QUALmode | (mode & TYPEPATmode), AnyRefClass.tpe)
-          )
-        )
-        tree setType ref1.tpe.resultType
+          }
+
+        if (!refTyped.isErrorTyped)
+          tree setType refTyped.tpe.resultType
+
+        if (treeInfo.admitsTypeSelection(refTyped)) tree
+        else UnstableTreeError(refTyped)
       }
 
       def typedSelectFromTypeTree(tree: SelectFromTypeTree) = {

src/interactive/scala/tools/nsc/interactive/Global.scala

@@ -925,14 +925,14 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
   }
 
   def stabilizedType(tree: Tree): Type = tree match {
-    case Ident(_) if tree.symbol.isStable =>
+    case Ident(_) if treeInfo.admitsTypeSelection(tree) =>
       singleType(NoPrefix, tree.symbol)
-    case Select(qual, _) if qual.tpe != null && tree.symbol.isStable =>
+    case Select(qual, _) if treeInfo.admitsTypeSelection(tree) =>
       singleType(qual.tpe, tree.symbol)
     case Import(expr, selectors) =>
       tree.symbol.info match {
         case analyzer.ImportType(expr) => expr match {
-          case s@Select(qual, name) => singleType(qual.tpe, s.symbol)
+          case s@Select(qual, name) if treeInfo.admitsTypeSelection(expr) => singleType(qual.tpe, s.symbol)
           case i : Ident => i.tpe
           case _ => tree.tpe
         }

src/interactive/scala/tools/nsc/interactive/Picklers.scala

@@ -96,7 +96,7 @@ trait Picklers { self: Global =>
       if (!sym.isRoot) {
         ownerNames(sym.owner, buf)
         buf += (if (sym.isModuleClass) sym.sourceModule else sym).name
-        if (!sym.isType && !sym.isStable) {
+        if (!sym.isType && !sym.isStable) { // TODO: what's the reasoning behind this condition!?
           val sym1 = sym.owner.info.decl(sym.name)
           if (sym1.isOverloaded) {
             val index = sym1.alternatives.indexOf(sym)

src/reflect/scala/reflect/internal/Symbols.scala

@@ -790,8 +790,12 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     /** Is this symbol an accessor method for outer? */
     final def isOuterField = isArtifact && (unexpandedName == nme.OUTER_LOCAL)
 
-    /** Does this symbol denote a stable value? */
-    def isStable = false
+    /** Does this symbol denote a stable value, ignoring volatility?
+     *
+     * Stability and volatility are checked separately to allow volatile paths in patterns that amount to equality checks. SI-6815
+     */
+    final def isStable        = isTerm && !isMutable && !(hasFlag(BYNAMEPARAM)) && (!isMethod || hasStableFlag)
+    final def hasVolatileType = tpe.isVolatile && !hasAnnotation(uncheckedStableClass)
 
     /** Does this symbol denote the primary constructor of its enclosing class? */
     final def isPrimaryConstructor =
@@ -2589,13 +2593,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     override def isMixinConstructor = name == nme.MIXIN_CONSTRUCTOR
     override def isConstructor      = nme.isConstructorName(name)
 
-    override def isPackageObject  = isModule && (name == nme.PACKAGE)
-    override def isStable = !isUnstable
-    private def isUnstable = (
-         isMutable
-      || (hasFlag(METHOD | BYNAMEPARAM) && !hasFlag(STABLE))
-      || (tpe.isVolatile && !hasAnnotation(uncheckedStableClass))
-    )
+    override def isPackageObject = isModule && (name == nme.PACKAGE)
 
     // The name in comments is what it is being disambiguated from.
     // TODO - rescue CAPTURED from BYNAMEPARAM so we can see all the names.

src/reflect/scala/reflect/internal/TreeGen.scala

@@ -142,17 +142,15 @@ abstract class TreeGen extends macros.TreeBuilder {
   }
 
   /** Computes stable type for a tree if possible */
-  def stableTypeFor(tree: Tree): Option[Type] = tree match {
-    case This(_) if tree.symbol != null && !tree.symbol.isError =>
-      Some(ThisType(tree.symbol))
-    case Ident(_) if tree.symbol.isStable =>
-      Some(singleType(tree.symbol.owner.thisType, tree.symbol))
-    case Select(qual, _) if ((tree.symbol ne null) && (qual.tpe ne null)) && // turned assert into guard for #4064
-                            tree.symbol.isStable && qual.tpe.isStable =>
-      Some(singleType(qual.tpe, tree.symbol))
-    case _ =>
-      None
-  }
+  def stableTypeFor(tree: Tree): Option[Type] =
+    if (treeInfo.admitsTypeSelection(tree))
+      tree match {
+        case This(_)         => Some(ThisType(tree.symbol))
+        case Ident(_)        => Some(singleType(tree.symbol.owner.thisType, tree.symbol))
+        case Select(qual, _) => Some(singleType(qual.tpe, tree.symbol))
+        case _               => None
+      }
+    else None
 
   /** Builds a reference with stable type to given symbol */
   def mkAttributedStableRef(pre: Type, sym: Symbol): Tree =

src/reflect/scala/reflect/internal/TreeInfo.scala

@@ -18,7 +18,7 @@ abstract class TreeInfo {
   val global: SymbolTable
 
   import global._
-  import definitions.{ isTupleSymbol, isVarArgsList, isCastSymbol, ThrowableClass, TupleClass, MacroContextClass, MacroContextPrefixType }
+  import definitions.{ isTupleSymbol, isVarArgsList, isCastSymbol, ThrowableClass, TupleClass, MacroContextClass, MacroContextPrefixType, uncheckedStableClass }
 
   /* Does not seem to be used. Not sure what it does anyway.
   def isOwnerDefinition(tree: Tree): Boolean = tree match {
@@ -66,6 +66,80 @@ abstract class TreeInfo {
       false
   }
 
+  /** Is `tree` a path, defined as follows? (Spec: 3.1 Paths)
+   *
+   * - The empty path ε (which cannot be written explicitly in user programs).
+   * - C.this, where C references a class.
+   * - p.x where p is a path and x is a stable member of p.
+   * - C.super.x or C.super[M].x where C references a class
+   *   and x references a stable member of the super class or designated parent class M of C.
+   *
+   * NOTE: Trees with errors are (mostly) excluded.
+   *
+   * Path ::= StableId | [id ‘.’] this
+   *
+   */
+  def isPath(tree: Tree, allowVolatile: Boolean): Boolean =
+    tree match {
+      // Super is not technically a path.
+      // However, syntactically, it can only occur nested in a Select.
+      // This gives a nicer definition of isStableIdentifier that's equivalent to the spec's.
+      // must consider Literal(_) a path for typedSingletonTypeTree
+      case EmptyTree | Literal(_) => true
+      case This(_) | Super(_, _)  => symOk(tree.symbol)
+      case _                      => isStableIdentifier(tree, allowVolatile)
+    }
+
+  /** Is `tree` a stable identifier, a path which ends in an identifier?
+   *
+   * StableId ::= id
+   *           | Path ‘.’ id
+   *           | [id ’.’] ‘super’ [‘[’ id ‘]’] ‘.’ id
+   */
+  def isStableIdentifier(tree: Tree, allowVolatile: Boolean): Boolean =
+    tree match {
+      case Ident(_)        => symOk(tree.symbol) && tree.symbol.isStable && !tree.symbol.hasVolatileType // TODO SPEC: not required by spec
+      case Select(qual, _) => isStableMemberOf(tree.symbol, qual, allowVolatile) && isPath(qual, allowVolatile)
+      case Apply(Select(free @ Ident(_), nme.apply), _) if free.symbol.name endsWith nme.REIFY_FREE_VALUE_SUFFIX =>
+        // see a detailed explanation of this trick in `GenSymbols.reifyFreeTerm`
+        free.symbol.hasStableFlag && isPath(free, allowVolatile)
+      case _               => false
+    }
+
+  private def symOk(sym: Symbol) = sym != null && !sym.isError && sym != NoSymbol
+  private def typeOk(tp: Type)   =  tp != null && ! tp.isError
+
+  /** Assuming `sym` is a member of `tree`, is it a "stable member"?
+   *
+   * Stable members are packages or members introduced
+   * by object definitions or by value definitions of non-volatile types (§3.6).
+   */
+  def isStableMemberOf(sym: Symbol, tree: Tree, allowVolatile: Boolean): Boolean = (
+    symOk(sym)       && (!sym.isTerm   || (sym.isStable && (allowVolatile || !sym.hasVolatileType))) &&
+    typeOk(tree.tpe) && (allowVolatile || !hasVolatileType(tree)) && !definitions.isByNameParamType(tree.tpe)
+  )
+
+  /** Is `tree`'s type volatile? (Ignored if its symbol has the @uncheckedStable annotation.)
+   */
+  def hasVolatileType(tree: Tree): Boolean =
+    symOk(tree.symbol) && tree.tpe.isVolatile && !tree.symbol.hasAnnotation(uncheckedStableClass)
+
+  /** Is `tree` either a non-volatile type,
+   *  or a path that does not include any of:
+   *   - a reference to a mutable variable/field
+   *   - a reference to a by-name parameter
+   *   - a member selection on a volatile type (Spec: 3.6 Volatile Types)?
+   *
+   * Such a tree is a suitable target for type selection.
+   */
+  def admitsTypeSelection(tree: Tree): Boolean = isPath(tree, allowVolatile = false)
+
+  /** Is `tree` admissible as a stable identifier pattern (8.1.5 Stable Identifier Patterns)?
+   *
+   * We disregard volatility, as it's irrelevant in patterns (SI-6815)
+   */
+  def isStableIdentifierPattern(tree: Tree): Boolean = isStableIdentifier(tree, allowVolatile = true)
+
   // TODO SI-5304 tighten this up so we don't elide side effect in module loads
   def isQualifierSafeToElide(tree: Tree): Boolean = isExprSafeToInline(tree)
 
@@ -473,9 +547,9 @@ abstract class TreeInfo {
   /** Does this CaseDef catch Throwable? */
   def catchesThrowable(cdef: CaseDef) = (
     cdef.guard.isEmpty && (unbind(cdef.pat) match {
-      case Ident(nme.WILDCARD)       => true
-      case i@Ident(name)             => hasNoSymbol(i)
-      case _                         => false
+      case Ident(nme.WILDCARD) => true
+      case i@Ident(name)       => hasNoSymbol(i)
+      case _                   => false
     })
   )