Fix #6199: Use a skolemized prefix in asSeenFrom when needed

Prior to
91ee02f,
unstable prefixes appearing in selection types were handled in a
two-pass approach which can be summarized as:

1. Type the selection with the unstable prefix, if this prefix appears
   in a position where it cannot be widened away, mark it with a special
   annotation.
2. If the result of the selection contains this annotation, retype it
   after having wrapped its prefix in a skolem.

This got replaced by the use of an `ApproximatingTypeMap` which can
always construct a valid (but potentially approximated) type for the
selection.

Most of the time this is all we need but there are some cases where
skolemizing the prefix is still useful as witnessed by the tests added
in this commit. They are now handled by unconditionally skolemizing
unstable prefixes.

To avoid any potential performance impact from this, we teach
`asSeenFrom` to always widen these prefixes first and only make use of
the skolem to avoid returning an approximation. Since this almost never
occurs (it happens only once when compiling Dotty) this means we incur
almost no extra cost (the skolem itself still needs to be allocated
unconditionally in advance, this cannot be delegated to `asSeenFrom`
because it's supposed to be an idempotent operation and each skolem is
unique).

Author: smarter

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

@@ -5,6 +5,7 @@ package core
 import Contexts._, Types._, Symbols._, Names._, Flags._
 import SymDenotations._
 import util.Spans._
+import util.Stats
 import util.SourcePosition
 import NameKinds.DepParamName
 import Decorators._
@@ -26,11 +27,34 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
   /** The type `tp` as seen from prefix `pre` and owner `cls`. See the spec
    *  for what this means.
    */
-  final def asSeenFrom(tp: Type, pre: Type, cls: Symbol): Type =
+  final def asSeenFrom(tp: Type, pre: Type, cls: Symbol): Type = {
+    pre match {
+      case pre: QualSkolemType =>
+        // When a selection has an unstable qualifier, the qualifier type gets
+        // wrapped in a `QualSkolemType` so that it may appear soundly as the
+        // prefix of a path in the selection type.
+        // However, we'd like to avoid referring to skolems when possible since
+        // they're an extra level of indirection we usually don't need, so we
+        // compute the type as seen from the widened prefix, and in the rare
+        // cases where this leads to an approximated type we recompute it with
+        // the skolemized prefix. See the i6199* tests for usecases.
+        val widenedAsf = new AsSeenFromMap(pre.info, cls)
+        val ret = widenedAsf.apply(tp)
+
+        if (!widenedAsf.approximated)
+          return ret
+
+        Stats.record("asSeenFrom skolem prefix required")
+      case _ =>
+    }
+
     new AsSeenFromMap(pre, cls).apply(tp)
+  }
 
   /** The TypeMap handling the asSeenFrom */
   class AsSeenFromMap(pre: Type, cls: Symbol) extends ApproximatingTypeMap {
+    /** Set to true when the result of `apply` was approximated to avoid an unstable prefix. */
+    var approximated: Boolean = false
 
     def apply(tp: Type): Type = {
 
@@ -44,7 +68,19 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
           case pre: SuperType => toPrefix(pre.thistpe, cls, thiscls)
           case _ =>
             if (thiscls.derivesFrom(cls) && pre.baseType(thiscls).exists)
-              if (variance <= 0 && !isLegalPrefix(pre)) range(defn.NothingType, pre)
+              if (variance <= 0 && !isLegalPrefix(pre)) {
+                if (variance < 0) {
+                  approximated = true
+                  defn.NothingType
+                }
+                else
+                  // Don't set the `approximated` flag yet: if this is a prefix
+                  // of a path, we might be able to dealias the path instead
+                  // (this is handled in `ApproximatingTypeMap`). If dealiasing
+                  // is not possible, then `expandBounds` will end up being
+                  // called which we override to set the `approximated` flag.
+                  range(defn.NothingType, pre)
+              }
               else pre
             else if ((pre.termSymbol is Package) && !(thiscls is Package))
               toPrefix(pre.select(nme.PACKAGE), cls, thiscls)
@@ -74,9 +110,14 @@ trait TypeOps { this: Context => // TODO: Make standalone object.
     override def reapply(tp: Type): Type =
       // derived infos have already been subjected to asSeenFrom, hence to need to apply the map again.
       tp
+
+    override protected def expandBounds(tp: TypeBounds): Type = {
+      approximated = true
+      super.expandBounds(tp)
+    }
   }
 
-  private def isLegalPrefix(pre: Type)(implicit ctx: Context) =
+  def isLegalPrefix(pre: Type)(implicit ctx: Context): Boolean =
     pre.isStable || !ctx.phase.isTyper
 
   /** Implementation of Types#simplified */

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

@@ -549,7 +549,7 @@ object Types {
      *  flags and no `excluded` flag and produce a denotation that contains
      *  the type of the member as seen from given prefix `pre`.
      */
-    final def findMember(name: Name, pre: Type, required: FlagConjunction, excluded: FlagSet)(implicit ctx: Context): Denotation = {
+    final def findMember(name: Name, pre: Type, required: FlagConjunction = EmptyFlagConjunction, excluded: FlagSet = EmptyFlags)(implicit ctx: Context): Denotation = {
       @tailrec def go(tp: Type): Denotation = tp match {
         case tp: TermRef =>
           go (tp.underlying match {
@@ -3723,6 +3723,21 @@ object Types {
     override def toString: String = s"Skolem($hashCode)"
   }
 
+  /** A skolem type used to wrap the type of the qualifier of a selection.
+   *
+   *  When typing a selection `e.f`, if `e` is unstable then we unconditionally
+   *  skolemize it. We use a subclass of `SkolemType` for this so that
+   *  [[TypeOps#asSeenFrom]] may treat it specially for optimization purposes,
+   *  see its implementation for more details.
+   */
+  class QualSkolemType(info: Type) extends SkolemType(info) {
+    override def derivedSkolemType(info: Type)(implicit ctx: Context): SkolemType =
+      if (info eq this.info) this else QualSkolemType(info)
+  }
+  object QualSkolemType {
+    def apply(info: Type): QualSkolemType = new QualSkolemType(info)
+  }
+
   // ------------ Type variables ----------------------------------------
 
   /** In a TypeApply tree, a TypeVar is created for each argument type to be inferred.
@@ -4646,6 +4661,9 @@ object Types {
       case _ => tp
     }
 
+    protected def expandBounds(tp: TypeBounds): Type =
+      range(atVariance(-variance)(reapply(tp.lo)), reapply(tp.hi))
+
     /** Try to widen a named type to its info relative to given prefix `pre`, where possible.
      *  The possible cases are listed inline in the code.
      */
@@ -4657,10 +4675,10 @@ object Types {
             // if H#T = U, then for any x in L..H, x.T =:= U,
             // hence we can replace with U under all variances
             reapply(alias.rewrapAnnots(tp1))
-          case TypeBounds(lo, hi) =>
+          case tp: TypeBounds =>
             // If H#T = _ >: S <: U, then for any x in L..H, S <: x.T <: U,
             // hence we can replace with S..U under all variances
-            range(atVariance(-variance)(reapply(lo)), reapply(hi))
+            expandBounds(tp)
           case info: SingletonType =>
             // if H#x: y.type, then for any x in L..H, x.type =:= y.type,
             // hence we can replace with y.type under all variances
@@ -4676,8 +4694,6 @@ object Types {
      *  underlying bounds to a range, otherwise return the expansion.
      */
     def expandParam(tp: NamedType, pre: Type): Type = {
-      def expandBounds(tp: TypeBounds) =
-        range(atVariance(-variance)(reapply(tp.lo)), reapply(tp.hi))
       tp.argForParam(pre) match {
         case arg @ TypeRef(pre, _) if pre.isArgPrefixOf(arg.symbol) =>
           arg.info match {

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -1016,14 +1016,14 @@ class TreeUnpickler(reader: TastyReader,
         val localCtx =
           if (name == nme.CONSTRUCTOR) ctx.addMode(Mode.InSuperCall) else ctx
         val qual = readTerm()(localCtx)
-        var pre = qual.tpe.widenIfUnstable
-        val denot = accessibleDenot(pre, name, sig)
+        var qualType = qual.tpe.widenIfUnstable
+        val denot = accessibleDenot(qualType, name, sig)
         val owner = denot.symbol.maybeOwner
-        if (owner.isPackageObject && pre.termSymbol.is(Package))
-          pre = pre.select(owner.sourceModule)
+        if (owner.isPackageObject && qualType.termSymbol.is(Package))
+          qualType = qualType.select(owner.sourceModule)
         val tpe = name match {
-          case name: TypeName => TypeRef(pre, name, denot)
-          case name: TermName => TermRef(pre, name, denot)
+          case name: TypeName => TypeRef(qualType, name, denot)
+          case name: TermName => TermRef(qualType, name, denot)
         }
         ConstFold(untpd.Select(qual, name).withType(tpe))
       }
@@ -1033,10 +1033,11 @@ class TreeUnpickler(reader: TastyReader,
          (untpd.Ident(qual.name).withSpan(qual.span), qual.tpe.asInstanceOf[TypeRef])
       }
 
-      def accessibleDenot(pre: Type, name: Name, sig: Signature) = {
-        val d = pre.member(name).atSignature(sig)
+      def accessibleDenot(qualType: Type, name: Name, sig: Signature) = {
+        val pre = ctx.typeAssigner.maybeSkolemizePrefix(qualType, name)
+        val d = qualType.findMember(name, pre).atSignature(sig)
         if (!d.symbol.exists || d.symbol.isAccessibleFrom(pre)) d
-        else pre.nonPrivateMember(name).atSignature(sig)
+        else qualType.findMember(name, pre, excluded = Private).atSignature(sig)
       }
 
       def readSimpleTerm(): Tree = tag match {

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

@@ -226,6 +226,17 @@ trait TypeAssigner {
     test(tpe, true)
   }
 
+  /** Return a potentially skolemized version of `qualTpe` to be used
+   *  as a prefix when selecting `name`.
+   *
+   *  @see QualSkolemType, TypeOps#asSeenFrom
+   */
+  def maybeSkolemizePrefix(qualType: Type, name: Name)(implicit ctx: Context): Type =
+    if (name.isTermName && !ctx.isLegalPrefix(qualType))
+      QualSkolemType(qualType)
+    else
+      qualType
+
   /** The type of the selection `tree`, where `qual1` is the typed qualifier part. */
   def selectionType(tree: untpd.RefTree, qual1: Tree)(implicit ctx: Context): Type = {
     var qualType = qual1.tpe.widenIfUnstable
@@ -234,8 +245,10 @@ trait TypeAssigner {
       qualType = errorType(em"$qualType takes type parameters", qual1.sourcePos)
     else if (!qualType.isInstanceOf[TermType])
       qualType = errorType(em"$qualType is illegal as a selection prefix", qual1.sourcePos)
+
     val name = tree.name
-    val mbr = qualType.member(name)
+    val pre = maybeSkolemizePrefix(qualType, name)
+    val mbr = qualType.findMember(name, pre)
     if (reallyExists(mbr))
       qualType.select(name, mbr)
     else if (qualType.derivesFrom(defn.DynamicClass) && name.isTermName && !Dynamic.isDynamicMethod(name))

tests/pos/i6199a.scala

@@ -0,0 +1,8 @@
+class Encoder[T] { def encode(v: T): String = v.toString }
+case class ValueWithEncoder[T](value: T, encoder: Encoder[T])
+
+object Test {
+  val a: Seq[ValueWithEncoder[_]] = Seq.empty
+  val b = a.map((value, encoder) => encoder.encode(value))
+  val c: Seq[String] = b
+}

tests/pos/i6199b.scala

@@ -0,0 +1,11 @@
+trait Foo {
+  type State
+  def bar(f: Bar[State] => Bar[State]): Foo = this
+}
+object Foo {
+  def unit: Foo = new Foo {
+    type State = Any
+  }
+  def doBar: Foo = unit.bar(bar => bar)
+}
+class Bar[+A]

tests/pos/i6199c.scala

@@ -0,0 +1,8 @@
+trait Foo[State] {
+  def bar(f: Bar[State] => Bar[State]): Foo[_] = this
+}
+object Foo {
+  def unit: Foo[_] = new Foo[Any] {}
+  def doBar: Foo[_] = unit.bar(bar => bar)
+}
+class Bar[+A]