Fix #4031: Check arguments of dependent methods for realizability

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

@@ -402,7 +402,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
         sym.owner.isPrimitiveValueClass || sym.owner == defn.StringClass
       if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol)
              // A constant expression with pure arguments is pure.
-          || fn.symbol.isStable)
+          || fn.symbol.isStableMember)
         minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
       else
         Impure
@@ -438,7 +438,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
     val sym = tree.symbol
     if (!tree.hasType) Impure
     else if (!tree.tpe.widen.isParameterless || sym.is(Erased)) SimplyPure
-    else if (!sym.isStable) Impure
+    else if (!sym.isStableMember) Impure
     else if (sym.is(Module))
       if (sym.moduleClass.isNoInitsClass) Pure else Idempotent
     else if (sym.is(Lazy)) Idempotent
@@ -504,7 +504,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
       case tpe: PolyType => maybeGetterType(tpe.resultType)
       case _ => false
     }
-    sym.owner.isClass && !sym.isStable && maybeGetterType(sym.info)
+    sym.owner.isClass && !sym.isStableMember && maybeGetterType(sym.info)
   }
 
   /** Is tree a reference to a mutable variable, or to a potential getter

compiler/src/dotty/tools/dotc/core/CheckRealizable.scala

@@ -14,7 +14,7 @@ import ast.tpd._
 /** Realizability status */
 object CheckRealizable {
 
-  abstract class Realizability(val msg: String) {
+  sealed abstract class Realizability(val msg: String) {
     def andAlso(other: => Realizability) =
       if (this == Realizable) other else this
     def mapError(f: Realizability => Realizability) =
@@ -70,17 +70,44 @@ class CheckRealizable(implicit ctx: Context) {
    */
   private def isLateInitialized(sym: Symbol) = sym.is(LateInitialized, butNot = Module)
 
-  /** The realizability status of given type `tp`*/
+  /** The realizability status of given type `tp`. We can only select members from realizable types.
+    * A type is realizable if it has non-null values. However, realizable types can
+    * have non-realizable subtypes, so we must restrict overriding. */
   def realizability(tp: Type): Realizability = tp.dealias match {
     case tp: TermRef =>
+      // Suppose tp is a.b.c.type, where c is declared with type T, then sym is c, tp.info is T and
+      // and tp.prefix is a.b.
       val sym = tp.symbol
+      // We know tp is realizable if either:
+      // 1. the symbol is stable and the prefix is realizable (so that, say, it contains no vars):
       if (sym.is(Stable)) realizability(tp.prefix)
       else {
+        // 2. if tp.info is a realizable singleton type. We check this last
+        // for performance, in all cases where some unrelated check might have failed.
+        def patchRealizability(r: Realizability) =
+          r.mapError(if (tp.info.isStableRealizable) Realizable else _)
         val r =
-          if (!sym.isStable) NotStable
-          else if (!isLateInitialized(sym)) realizability(tp.prefix)
-          else if (!sym.isEffectivelyFinal) new NotFinal(sym)
-          else realizability(tp.info).mapError(r => new ProblemInUnderlying(tp.info, r))
+          // Reject fields that are mutable, by-name, and similar.
+          if (!sym.isStableMember)
+            patchRealizability(NotStable)
+            // 3. If the symbol isn't "lazy" and its prefix is realizable
+          else if (!isLateInitialized(sym)) {
+            // The symbol itself is stable, cache this information:
+            sym.setFlag(Stable)
+            // Realizability now depends on the prefix:
+            patchRealizability(realizability(tp.prefix).mapError(r => new ProblemInUnderlying(tp.prefix, r)))
+          } else if (!sym.isEffectivelyFinal)
+            patchRealizability(new NotFinal(sym))
+          else
+            // 4. If the symbol is effectively final, and a lazy or erased val
+            // and has a realizable type. We require finality because overrides
+            // of realizable fields might not be realizable.
+
+            // Since patchRealizability checks realizability(tp.info) through
+            // isStableRealizable, using patchRealizability wouldn't make
+            // a difference, and calling it here again might introduce
+            // a slowdown exponential in the prefix length.
+            realizability(tp.info).mapError(r => new ProblemInUnderlying(tp.info, r))
         if (r == Realizable) sym.setFlag(Stable)
         r
       }

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -589,12 +589,14 @@ object SymDenotations {
       )
 
     /** Is this a denotation of a stable term (or an arbitrary type)? */
-    final def isStable(implicit ctx: Context) =
-      isType || is(StableOrErased) || !is(UnstableValue) && !info.isInstanceOf[ExprType]
+    final def isStableMember(implicit ctx: Context) = {
+      def isUnstableValue = is(UnstableValue) || info.isInstanceOf[ExprType]
+      isType || is(StableOrErased) || !isUnstableValue
+    }
 
     /** Is this a denotation of a class that does not have - either direct or inherited -
-     *  initaliazion code?
-     */
+      *  initaliazion code?
+      */
     def isNoInitsClass(implicit ctx: Context) =
       isClass && asClass.baseClasses.forall(_.is(NoInits))
 

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

@@ -302,7 +302,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
         thirdTry
       case tp1: TypeParamRef =>
         def flagNothingBound = {
-          if (!frozenConstraint && tp2.isRef(defn.NothingClass) && state.isGlobalCommittable) {
+          if (!frozenConstraint && tp2.isRef(NothingClass) && state.isGlobalCommittable) {
             def msg = s"!!! instantiated to Nothing: $tp1, constraint = ${constraint.show}"
             if (Config.failOnInstantiationToNothing) assert(false, msg)
             else ctx.log(msg)
@@ -384,8 +384,9 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
         if (cls2.isClass) {
           if (cls2.typeParams.isEmpty) {
             if (cls2 eq AnyKindClass) return true
-            if (tp1.isRef(defn.NothingClass)) return true
+            if (tp1.isRef(NothingClass)) return true
             if (tp1.isLambdaSub) return false
+            if (cls2 eq AnyClass) return true
               // Note: We would like to replace this by `if (tp1.hasHigherKind)`
               // but right now we cannot since some parts of the standard library rely on the
               // idiom that e.g. `List <: Any`. We have to bootstrap without scalac first.
@@ -399,7 +400,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
             val base = tp1.baseType(cls2)
             if (base.typeSymbol == cls2) return true
           }
-          else if (tp1.isLambdaSub && !tp1.isRef(defn.AnyKindClass))
+          else if (tp1.isLambdaSub && !tp1.isRef(AnyKindClass))
             return recur(tp1, EtaExpansion(cls2.typeRef))
         }
         fourthTry
@@ -1296,7 +1297,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
                       // at run time. It would not work to replace that with `Nothing`.
                       // However, maybe we can still apply the replacement to
                       // types which are not explicitly written.
-                      defn.NothingType
+                      NothingType
                     case _ => andType(tp1, tp2)
                   }
                 case _ => andType(tp1, tp2)
@@ -1307,8 +1308,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
   }
 
   /** The greatest lower bound of a list types */
-  final def glb(tps: List[Type]): Type =
-    ((defn.AnyType: Type) /: tps)(glb)
+  final def glb(tps: List[Type]): Type = ((AnyType: Type) /: tps)(glb)
 
   /** The least upper bound of two types
    *  @param canConstrain  If true, new constraints might be added to simplify the lub.
@@ -1338,7 +1338,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
 
   /** The least upper bound of a list of types */
   final def lub(tps: List[Type]): Type =
-    ((defn.NothingType: Type) /: tps)(lub(_,_, canConstrain = false))
+    ((NothingType: Type) /: tps)(lub(_,_, canConstrain = false))
 
   /** Try to produce joint arguments for a lub `A[T_1, ..., T_n] | A[T_1', ..., T_n']` using
    *  the following strategies:

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -19,6 +19,7 @@ import Periods._
 import util.Positions.{Position, NoPosition}
 import util.Stats._
 import util.{DotClass, SimpleIdentitySet}
+import CheckRealizable._
 import reporting.diagnostic.Message
 import ast.tpd._
 import ast.TreeTypeMap
@@ -150,14 +151,20 @@ object Types {
 
     /** Does this type denote a stable reference (i.e. singleton type)? */
     final def isStable(implicit ctx: Context): Boolean = stripTypeVar match {
-      case tp: TermRef => tp.termSymbol.isStable && tp.prefix.isStable || tp.info.isStable
+      case tp: TermRef => tp.termSymbol.isStableMember && tp.prefix.isStable || tp.info.isStable
       case _: SingletonType | NoPrefix => true
       case tp: RefinedOrRecType => tp.parent.isStable
       case tp: ExprType => tp.resultType.isStable
       case tp: AnnotatedType => tp.parent.isStable
       case _ => false
     }
 
+    /** Does this type denote a realizable stable reference? Much more expensive to check
+     *  than isStable, that's why some of the checks are done later in PostTyper.
+     */
+    final def isStableRealizable(implicit ctx: Context): Boolean =
+      isStable && realizability(this) == Realizable
+
     /** Is this type a (possibly refined or applied or aliased) type reference
      *  to the given type symbol?
      *  @sym  The symbol to compare to. It must be a class symbol or abstract type.
@@ -985,7 +992,7 @@ object Types {
     /** Widen type if it is unstable (i.e. an ExprType, or TermRef to unstable symbol */
     final def widenIfUnstable(implicit ctx: Context): Type = stripTypeVar match {
       case tp: ExprType => tp.resultType.widenIfUnstable
-      case tp: TermRef if !tp.symbol.isStable => tp.underlying.widenIfUnstable
+      case tp: TermRef if !tp.symbol.isStableMember => tp.underlying.widenIfUnstable
       case _ => this
     }
 
@@ -4190,6 +4197,8 @@ object Types {
       else if (variance < 0) lo
       else Range(lower(lo), upper(hi))
 
+    protected def emptyRange = range(defn.NothingType, defn.AnyType)
+
     protected def isRange(tp: Type) = tp.isInstanceOf[Range]
 
     protected def lower(tp: Type) = tp match {
@@ -4306,7 +4315,7 @@ object Types {
       else tp.derivedTypeBounds(lo, hi)
 
     override protected def derivedSuperType(tp: SuperType, thistp: Type, supertp: Type) =
-      if (isRange(thistp) || isRange(supertp)) range(defn.NothingType, defn.AnyType)
+      if (isRange(thistp) || isRange(supertp)) emptyRange
       else tp.derivedSuperType(thistp, supertp)
 
     override protected def derivedAppliedType(tp: AppliedType, tycon: Type, args: List[Type]): Type =

compiler/src/dotty/tools/dotc/sbt/ExtractAPI.scala

@@ -328,7 +328,7 @@ private class ExtractAPICollector(implicit val ctx: Context) extends ThunkHolder
     } else if (sym.is(Mutable, butNot = Accessor)) {
       api.Var.of(sym.name.toString, apiAccess(sym), apiModifiers(sym),
         apiAnnotations(sym).toArray, apiType(sym.info))
-    } else if (sym.isStable && !sym.isRealMethod) {
+    } else if (sym.isStableMember && !sym.isRealMethod) {
       api.Val.of(sym.name.toString, apiAccess(sym), apiModifiers(sym),
         apiAnnotations(sym).toArray, apiType(sym.info))
     } else {

compiler/src/dotty/tools/dotc/transform/Getters.scala

@@ -64,7 +64,7 @@ class Getters extends MiniPhase with SymTransformer {
 
     var d1 =
       if (d.isTerm && (d.is(Lazy) || d.owner.isClass) && d.info.isValueType && !noGetterNeeded) {
-        val maybeStable = if (d.isStable) Stable else EmptyFlags
+        val maybeStable = if (d.isStableMember) Stable else EmptyFlags
         d.copySymDenotation(
           initFlags = d.flags | maybeStable | AccessorCreationFlags,
           info = ExprType(d.info))

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -434,8 +434,9 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
           def addTyped(arg: Arg, formal: Type): Type => Type = {
             addArg(typedArg(arg, formal), formal)
             if (methodType.isParamDependent)
-              safeSubstParam(_, methodType.paramRefs(n), typeOfArg(arg))
-            else identity
+              safeSubstParam(_, methodType.paramRefs(n), typeOfArg(arg), initVariance = -1)
+            else
+              identity
           }
 
           def missingArg(n: Int): Unit = {

compiler/src/dotty/tools/dotc/typer/Inliner.scala

@@ -747,7 +747,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
         case tpe: NamedType if tpe.symbol.exists && !tpe.symbol.isAccessibleFrom(tpe.prefix, superAccess) =>
           tpe.info match {
             case TypeAlias(alias) => return ensureAccessible(alias, superAccess, pos)
-            case info: ConstantType if tpe.symbol.isStable => return info
+            case info: ConstantType if tpe.symbol.isStableMember => return info
             case _ =>
           }
         case _ =>

compiler/src/dotty/tools/dotc/typer/PrepareTransparent.scala

@@ -81,7 +81,7 @@ object PrepareTransparent {
         sym.isTerm &&
         (sym.is(AccessFlags) || sym.privateWithin.exists) &&
         !sym.isContainedIn(inlineSym) &&
-        !(sym.isStable && sym.info.widenTermRefExpr.isInstanceOf[ConstantType])
+        !(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType])
 
       def preTransform(tree: Tree)(implicit ctx: Context): Tree
 
@@ -438,4 +438,4 @@ object PrepareTransparent {
     val untpdSplice = tpd.UntypedSplice(addRefs.transform(original)).withType(typed.tpe)
     seq(implicitBindings, untpdSplice)
   }
-}
+}

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -367,7 +367,7 @@ object RefChecks {
         intersectionIsEmpty(member.extendedOverriddenSymbols, other.extendedOverriddenSymbols)) {
         overrideError("cannot override a concrete member without a third member that's overridden by both " +
           "(this rule is designed to prevent ``accidental overrides'')")
-      } else if (other.isStable && !member.isStable) { // (1.4)
+      } else if (other.isStableMember && !member.isStableMember) { // (1.4)
         overrideError("needs to be a stable, immutable value")
       } else if (member.is(ModuleVal) && !other.isRealMethod && !other.is(Deferred | Lazy)) {
         overrideError("may not override a concrete non-lazy value")
@@ -963,7 +963,7 @@ class RefChecks extends MiniPhase { thisPhase =>
     checkAllOverrides(cls)
     tree
   } catch {
-    case ex: MergeError =>
+    case ex: TypeError =>
       ctx.error(ex.getMessage, tree.pos)
       tree
   }

compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala

@@ -14,6 +14,7 @@ import collection.mutable
 import reporting.diagnostic.Message
 import reporting.diagnostic.messages._
 import Checking.checkNoPrivateLeaks
+import CheckRealizable._
 
 trait TypeAssigner {
   import tpd._
@@ -106,7 +107,7 @@ trait TypeAssigner {
             case info: ClassInfo =>
               range(defn.NothingType, apply(classBound(info)))
             case _ =>
-              range(defn.NothingType, defn.AnyType) // should happen only in error cases
+              emptyRange // should happen only in error cases
           }
         case tp: ThisType if toAvoid(tp.cls) =>
           range(defn.NothingType, apply(classBound(tp.cls.classInfo)))
@@ -340,13 +341,31 @@ trait TypeAssigner {
     }
   }
 
-  /** Substitute argument type `argType` for parameter `pref` in type `tp`,
-   *  skolemizing the argument type if it is not stable and `pref` occurs in `tp`.
+  /** Substitute argument type `argType` for parameter `pref` in type `tp`, but
+   *  take special measures if the argument is not realizable:
+   *  1. If the widened argument type is known to have good bounds,
+   *     substitute the skolemized argument type.
+   *  2. If the widened argument type is not known to have good bounds, eliminate all references
+   *     to the parameter in `tp`.
+   *  (2) is necessary since even with a skolemized type we might break subtyping if
+   *  bounds are bad. This could lead to errors not being detected. A test case is the second
+   *  failure in neg/i4031.scala
    */
-  def safeSubstParam(tp: Type, pref: ParamRef, argType: Type)(implicit ctx: Context) = {
+  def safeSubstParam(tp: Type, pref: ParamRef, argType: Type, initVariance: Int = 1)(implicit ctx: Context) = {
     val tp1 = tp.substParam(pref, argType)
-    if ((tp1 eq tp) || argType.isStable) tp1
-    else tp.substParam(pref, SkolemType(argType.widen))
+    if ((tp1 eq tp) || argType.isStableRealizable) tp1
+    else {
+      val widenedArgType = argType.widen
+      if (realizability(widenedArgType) == Realizable)
+        tp.substParam(pref, SkolemType(widenedArgType))
+      else {
+        val avoidParam = new ApproximatingTypeMap {
+          variance = initVariance
+          def apply(t: Type) = if (t `eq` pref) emptyRange else mapOver(t)
+        }
+        avoidParam(tp)
+      }
+    }
   }
 
   /** Substitute types of all arguments `args` for corresponding `params` in `tp`.

tests/neg/i4031-posttyper.scala

@@ -0,0 +1,7 @@
+object App {
+  trait A { type L >: Any}
+  def upcast(a: A, x: Any): a.L = x
+  lazy val p: A { type L <: Nothing } = p
+  val q = new A { type L = Any }
+  def coerce2(x: Any): Int = upcast(p, x): p.L // error: not a legal path
+}

tests/neg/i4031.scala

@@ -0,0 +1,17 @@
+object App {
+  trait A { type L >: Any}
+  def upcast(a: A, x: Any): a.L = x
+  lazy val p: A { type L <: Nothing } = p
+  val q = new A { type L = Any }
+  def coerce(x: Any): Int = upcast(p, x)  // error: not a legal path
+
+  def compare(x: A, y: x.L) = assert(x == y)
+  def compare2(x: A)(y: x.type) = assert(x == y)
+
+
+  def main(args: Array[String]): Unit = {
+    println(coerce("Uh oh!"))
+    compare(p, p) // error: not a legal path
+    compare2(p)(p) // error: not a legal path
+  }
+}

tests/neg/i50-volatile.scala

@@ -12,14 +12,21 @@ class Test {
 
   class Client extends o.Inner                          // old-error // old-error
 
-  def xToString(x: o.X): String = x                     // old-error
+  def xToString(x: o.X): String = x                     // error
 
   def intToString(i: Int): String = xToString(i)
 }
-object Test2 {
 
-  import Test.o._                                       // error
+object Test2 {
+  trait A {
+    type X = String
+  }
+  trait B {
+    type X = Int
+  }
+  lazy val o: A & B = ???
 
-  def xToString(x: X): String = x
+  def xToString(x: o.X): String = x       // error
 
+  def intToString(i: Int): String = xToString(i)
 }