Merge pull request #5287 from dotty-staging/fix-#5279

Fix #5279: Be more careful where we create symbolic refs from named ones

Author: odersky

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

@@ -1917,6 +1917,10 @@ object Types {
         ||
         lastSymbol.infoOrCompleter.isInstanceOf[ErrorType]
         ||
+        !sym.exists
+        ||
+        !lastSymbol.exists
+        ||
         sym.isPackageObject // package objects can be visited before we get around to index them
         ||
         sym.owner != lastSymbol.owner &&
@@ -2079,17 +2083,36 @@ object Types {
       else this
 
     /** A reference like this one, but with the given denotation, if it exists.
-     *  If the symbol of `denot` is the same as the current symbol, the denotation
-     *  is re-used, otherwise a new one is created.
+     *  Returns a new named type with the denotation's symbol if that symbol exists, and
+     *  one of the following alternatives applies:
+     *   1. The current designator is a symbol and the symbols differ, or
+     *   2. The current designator is a name and the new symbolic named type
+     *      does not have a currently known denotation.
+     *   3. The current designator is a name and the new symbolic named type
+     *      has the same info as the current info
+     *  Otherwise the current denotation is overwritten with the given one.
+     *
+     *  Note: (2) and (3) are a "lock in mechanism" where a reference with a name as
+     *  designator can turn into a symbolic reference.
+     *
+     *  Note: This is a subtle dance to keep the balance between going to symbolic
+     *  references as much as we can (since otherwise we'd risk getting cycles)
+     *  and to still not lose any type info in the denotation (since symbolic
+     *  references often recompute their info directly from the symbol's info).
+     *  A test case is neg/opaque-self-encoding.scala.
      */
     final def withDenot(denot: Denotation)(implicit ctx: Context): ThisType =
       if (denot.exists) {
         val adapted = withSym(denot.symbol)
-        if (adapted ne this) adapted.withDenot(denot).asInstanceOf[ThisType]
-        else {
-          setDenot(denot)
-          this
-        }
+        val result =
+          if (adapted.eq(this)
+              || designator.isInstanceOf[Symbol]
+              || !adapted.denotationIsCurrent
+              || adapted.info.eq(denot.info))
+            adapted
+          else this
+        result.setDenot(denot)
+        result.asInstanceOf[ThisType]
       }
       else // don't assign NoDenotation, we might need to recover later. Test case is pos/avoid.scala.
         this
@@ -2190,6 +2213,11 @@ object Types {
     override protected def designator_=(d: Designator): Unit = myDesignator = d
 
     override def underlying(implicit ctx: Context): Type = info
+
+    /** Hook that can be called from creation methods in TermRef and TypeRef */
+    def validated(implicit ctx: Context): this.type = {
+      this
+    }
   }
 
   final class CachedTermRef(prefix: Type, designator: Designator, hc: Int) extends TermRef(prefix, designator) {
@@ -2206,6 +2234,23 @@ object Types {
   private def assertUnerased()(implicit ctx: Context) =
     if (Config.checkUnerased) assert(!ctx.phase.erasedTypes)
 
+  /** The designator to be used for a named type creation with given prefix, name, and denotation.
+   *  This is the denotation's symbol, if it exists and the prefix is not the this type
+   *  of the class owning the symbol. The reason for the latter qualification is that
+   *  when re-computing the denotation of a `this.<symbol>` reference we read the
+   *  type directly off the symbol. But the given denotation might contain a more precise
+   *  type than what can be computed from the symbol's info. We have to create in this case
+   *  a reference with a name as designator so that the denotation will be correctly updated in
+   *  the future. See also NamedType#withDenot. Test case is neg/opaque-self-encoding.scala.
+   */
+  private def designatorFor(prefix: Type, name: Name, denot: Denotation)(implicit ctx: Context): Designator = {
+    val sym = denot.symbol
+    if (sym.exists && (prefix.eq(NoPrefix) || prefix.ne(sym.owner.thisType)))
+      sym
+    else
+      name
+  }
+
   object NamedType {
     def isType(desig: Designator)(implicit ctx: Context): Boolean = desig match {
       case sym: Symbol => sym.isType
@@ -2229,7 +2274,7 @@ object Types {
      *  from the denotation's symbol if the latter exists, or else it is the given name.
      */
     def apply(prefix: Type, name: TermName, denot: Denotation)(implicit ctx: Context): TermRef =
-      apply(prefix, if (denot.symbol.exists) denot.symbol.asTerm else name).withDenot(denot)
+      apply(prefix, designatorFor(prefix, name, denot)).withDenot(denot)
   }
 
   object TypeRef {
@@ -2242,7 +2287,7 @@ object Types {
      *  from the denotation's symbol if the latter exists, or else it is the given name.
      */
     def apply(prefix: Type, name: TypeName, denot: Denotation)(implicit ctx: Context): TypeRef =
-      apply(prefix, if (denot.symbol.exists) denot.symbol.asType else name).withDenot(denot)
+      apply(prefix, designatorFor(prefix, name, denot)).withDenot(denot)
   }
 
   // --- Other SingletonTypes: ThisType/SuperType/ConstantType ---------------------------

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

@@ -185,16 +185,16 @@ class TreePickler(pickler: TastyPickler) {
         writeByte(if (tpe.isType) TYPEREFdirect else TERMREFdirect)
         pickleSymRef(sym)
       }
-      else if (isLocallyDefined(sym)) {
-        writeByte(if (tpe.isType) TYPEREFsymbol else TERMREFsymbol)
-        pickleSymRef(sym); pickleType(tpe.prefix)
-      }
       else tpe.designator match {
         case name: Name =>
           writeByte(if (tpe.isType) TYPEREF else TERMREF)
           pickleName(name); pickleType(tpe.prefix)
         case sym: Symbol =>
-          pickleExternalRef(sym)
+          if (isLocallyDefined(sym)) {
+            writeByte(if (tpe.isType) TYPEREFsymbol else TERMREFsymbol)
+            pickleSymRef(sym); pickleType(tpe.prefix)
+          }
+          else pickleExternalRef(sym)
       }
     case tpe: ThisType =>
       if (tpe.cls.is(Flags.Package) && !tpe.cls.isEffectiveRoot) {

tests/neg/opaque-self-encoding.scala

@@ -0,0 +1,10 @@
+trait TS { type TX = Int }
+trait TT { self: { type TX = Int } =>
+  type TX
+  def lift(x: Int): TX = x
+}
+
+class Test {
+  val t = new TT {}
+  t.lift(1): Int // error
+}

tests/pos/refinements.scala

@@ -0,0 +1,15 @@
+trait TS { type TX = Int }
+trait TT { self: { type TX = Int } =>
+  type TX
+  def lift(x: Int): TX = x
+}
+
+// A more direct version
+
+trait UU {
+  type UX
+  val u: UX
+  val x: this.type & { type UX = Int }
+  val y: Int = x.u
+  val z: x.UX = y
+}