Merge pull request scala#7166 from dotty-staging/fix-#7159

Fix scala#7159: Add missing asSeenFrom when updating denotations

Author: nicolasstucki

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

@@ -490,7 +490,7 @@ object Denotations {
                   // compare with way merge is performed in SymDenotation#computeMembersNamed
                   else throw new MergeError(ex.sym1, ex.sym2, ex.tp1, ex.tp2, pre)
               }
-            new JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor)
+            new JointRefDenotation(sym, jointInfo, denot1.validFor & denot2.validFor, pre)
           }
         }
       }
@@ -543,7 +543,7 @@ object Denotations {
                 lubSym(sym1.allOverriddenSymbols, NoSymbol)
               }
             new JointRefDenotation(
-                jointSym, infoJoin(info1, info2, sym1, sym2), denot1.validFor & denot2.validFor)
+                jointSym, infoJoin(info1, info2, sym1, sym2), denot1.validFor & denot2.validFor, pre)
           }
         }
         else NoDenotation
@@ -716,10 +716,15 @@ object Denotations {
 
   /** A non-overloaded denotation */
   abstract class SingleDenotation(symbol: Symbol, initInfo: Type) extends Denotation(symbol, initInfo) {
-    protected def newLikeThis(symbol: Symbol, info: Type): SingleDenotation
+    protected def newLikeThis(symbol: Symbol, info: Type, pre: Type): SingleDenotation
 
     final def name(implicit ctx: Context): Name = symbol.name
 
+    /** If this is not a SymDenotation: The prefix under which the denotation was constructed.
+     *  NoPrefix for SymDenotations.
+     */
+    def prefix: Type = NoPrefix
+
     final def signature(implicit ctx: Context): Signature =
       if (isType) Signature.NotAMethod // don't force info if this is a type SymDenotation
       else info match {
@@ -733,9 +738,9 @@ object Denotations {
         case _ => Signature.NotAMethod
       }
 
-    def derivedSingleDenotation(symbol: Symbol, info: Type)(implicit ctx: Context): SingleDenotation =
-      if ((symbol eq this.symbol) && (info eq this.info)) this
-      else newLikeThis(symbol, info)
+    def derivedSingleDenotation(symbol: Symbol, info: Type, pre: Type = this.prefix)(implicit ctx: Context): SingleDenotation =
+      if ((symbol eq this.symbol) && (info eq this.info) && (pre eq this.prefix)) this
+      else newLikeThis(symbol, info, pre)
 
     def mapInfo(f: Type => Type)(implicit ctx: Context): SingleDenotation =
       derivedSingleDenotation(symbol, f(info))
@@ -1126,9 +1131,29 @@ object Denotations {
         case thisd: SymDenotation => thisd.owner
         case _ => if (symbol.exists) symbol.owner else NoSymbol
       }
-      def derived(info: Type) = derivedSingleDenotation(symbol, info.asSeenFrom(pre, owner))
+
+      /** The derived denotation with the given `info` transformed with `asSeenFrom`.
+       *  The prefix of the derived denotation is the new prefix `pre` if the type is
+       *  opaque, or if the current prefix is already different from `NoPrefix`.
+       *  That leaves SymDenotations (which have NoPrefix as the prefix), which are left
+       *  as SymDenotations unless the type is opaque. The treatment of opaque types
+       *  is needed, without it i7159.scala fails in from-tasty. Without the treatment,
+       *  opaque type denotations in subclasses are kept as SymDenotations, which means
+       *  that the transform in `ElimOpaque` will return the symbol's opaque alias without
+       *  adding the needed asSeenFrom.
+       *
+       *  Logically, the right thing to do would be to extend the same treatment to all denotations
+       *  Currently this fails the bootstrap. There's also a concern that this generalization
+       *  would create more denotation objects, at a price in performance.
+       */
+      def derived(info: Type) =
+        derivedSingleDenotation(
+          symbol,
+          info.asSeenFrom(pre, owner),
+          if (symbol.is(Opaque) || this.prefix != NoPrefix) pre else this.prefix)
+
       pre match {
-        case pre: ThisType if symbol.isOpaqueAlias && pre.cls == symbol.owner =>
+        case pre: ThisType if symbol.isOpaqueAlias && pre.cls == owner =>
           // This code is necessary to compensate for a "window of vulnerability" with
           // opaque types. The problematic sequence is as follows.
           //  1. Type a selection  `m.this.T` where `T` is an opaque type alias in `m`
@@ -1164,34 +1189,39 @@ object Denotations {
     }
   }
 
-  abstract class NonSymSingleDenotation(symbol: Symbol, initInfo: Type) extends SingleDenotation(symbol, initInfo) {
+  abstract class NonSymSingleDenotation(symbol: Symbol, initInfo: Type, override val prefix: Type) extends SingleDenotation(symbol, initInfo) {
     def infoOrCompleter: Type = initInfo
     def isType: Boolean = infoOrCompleter.isInstanceOf[TypeType]
   }
 
   class UniqueRefDenotation(
     symbol: Symbol,
     initInfo: Type,
-    initValidFor: Period) extends NonSymSingleDenotation(symbol, initInfo) {
+    initValidFor: Period,
+    prefix: Type) extends NonSymSingleDenotation(symbol, initInfo, prefix) {
     validFor = initValidFor
     override def hasUniqueSym: Boolean = true
-    protected def newLikeThis(s: Symbol, i: Type): SingleDenotation = new UniqueRefDenotation(s, i, validFor)
+    protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
+      new UniqueRefDenotation(s, i, validFor, pre)
   }
 
   class JointRefDenotation(
     symbol: Symbol,
     initInfo: Type,
-    initValidFor: Period) extends NonSymSingleDenotation(symbol, initInfo) {
+    initValidFor: Period,
+    prefix: Type) extends NonSymSingleDenotation(symbol, initInfo, prefix) {
     validFor = initValidFor
     override def hasUniqueSym: Boolean = false
-    protected def newLikeThis(s: Symbol, i: Type): SingleDenotation = new JointRefDenotation(s, i, validFor)
+    protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
+      new JointRefDenotation(s, i, validFor, pre)
   }
 
-  class ErrorDenotation(implicit ctx: Context) extends NonSymSingleDenotation(NoSymbol, NoType) {
+  class ErrorDenotation(implicit ctx: Context) extends NonSymSingleDenotation(NoSymbol, NoType, NoType) {
     override def exists: Boolean = false
     override def hasUniqueSym: Boolean = false
     validFor = Period.allInRun(ctx.runId)
-    protected def newLikeThis(s: Symbol, i: Type): SingleDenotation = this
+    protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
+      this
   }
 
   /** An error denotation that provides more info about the missing reference.

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

@@ -1396,7 +1396,8 @@ object SymDenotations {
 
     // ----- copies and transforms  ----------------------------------------
 
-    protected def newLikeThis(s: Symbol, i: Type): SingleDenotation = new UniqueRefDenotation(s, i, validFor)
+    protected def newLikeThis(s: Symbol, i: Type, pre: Type): SingleDenotation =
+      new UniqueRefDenotation(s, i, validFor, pre)
 
     /** Copy this denotation, overriding selective fields */
     final def copySymDenotation(

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

@@ -663,7 +663,7 @@ object Types {
         }
         else
           pdenot & (
-            new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId)),
+            new JointRefDenotation(NoSymbol, rinfo, Period.allInRun(ctx.runId), pre),
             pre,
             safeIntersection = ctx.base.pendingMemberSearches.contains(name))
       }
@@ -701,7 +701,7 @@ object Types {
       def goSuper(tp: SuperType) = go(tp.underlying) match {
         case d: JointRefDenotation =>
           typr.println(i"redirecting super.$name from $tp to ${d.symbol.showLocated}")
-          new UniqueRefDenotation(d.symbol, tp.memberInfo(d.symbol), d.validFor)
+          new UniqueRefDenotation(d.symbol, tp.memberInfo(d.symbol), d.validFor, pre)
         case d => d
       }
 
@@ -4092,7 +4092,8 @@ object Types {
         // Note: Taking a normal typeRef does not work here. A normal ref might contain
         // also other information about the named type (e.g. bounds).
         contains(
-          TypeRef(tp.prefix, cls).withDenot(new UniqueRefDenotation(cls, tp, cls.validFor)))
+          TypeRef(tp.prefix, cls)
+            .withDenot(new UniqueRefDenotation(cls, tp, cls.validFor, tp.prefix)))
       case _ =>
         lo <:< tp && tp <:< hi
     }

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

@@ -8,7 +8,7 @@ import Types._
 import Contexts.Context
 import Symbols._
 import Decorators._
-import Denotations.SingleDenotation
+import Denotations.{SingleDenotation, NonSymSingleDenotation}
 import SymDenotations.SymDenotation
 import DenotTransformers._
 import TypeUtils._
@@ -46,11 +46,9 @@ class ElimOpaque extends MiniPhase with DenotTransformer {
         ref.copySymDenotation(
           info = cinfo.derivedClassInfo(selfInfo = strippedSelfType),
           initFlags = ref.flags &~ Opaque)
+      case ref: NonSymSingleDenotation if sym.isOpaqueAlias =>
+        ref.derivedSingleDenotation(sym, TypeAlias(sym.opaqueAlias.asSeenFrom(ref.prefix, sym.owner)))
       case _ =>
-        // This is dubious as it means that we do not see the alias from an external reference
-        // which has type UniqueRefDenotation instead of SymDenotation. But to correctly recompute
-        // the alias we'd need a prefix, which we do not have here. To fix this, we'd have to
-        // maintain a prefix in UniqueRefDenotations and JointRefDenotations.
         ref
     }
   }

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

@@ -89,7 +89,7 @@ class Erasure extends Phase with DenotTransformer {
       }
     case ref: JointRefDenotation =>
       new UniqueRefDenotation(
-        ref.symbol, transformInfo(ref.symbol, ref.symbol.info), ref.validFor)
+        ref.symbol, transformInfo(ref.symbol, ref.symbol.info), ref.validFor, ref.prefix)
     case _ =>
       ref.derivedSingleDenotation(ref.symbol, transformInfo(ref.symbol, ref.symbol.info))
   }

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

@@ -501,13 +501,22 @@ class TreeChecker extends Phase with SymTransformer {
     override def adapt(tree: Tree, pt: Type, locked: TypeVars)(implicit ctx: Context): Tree = {
       def isPrimaryConstructorReturn =
         ctx.owner.isPrimaryConstructor && pt.isRef(ctx.owner.owner) && tree.tpe.isRef(defn.UnitClass)
+      def infoStr(tp: Type) = tp match {
+        case tp: TypeRef =>
+          val sym = tp.symbol
+          i"${sym.showLocated} with ${tp.designator}, flags = ${sym.flagsString}, underlying = ${tp.underlyingIterator.toList}%, %"
+        case _ =>
+          "??"
+      }
       if (ctx.mode.isExpr &&
           !tree.isEmpty &&
           !isPrimaryConstructorReturn &&
           !pt.isInstanceOf[FunOrPolyProto])
         assert(tree.tpe <:< pt, {
           val mismatch = err.typeMismatchMsg(tree.tpe, pt)
           i"""|${mismatch.msg}
+              |found: ${infoStr(tree.tpe)}
+              |expected: ${infoStr(pt)}
               |tree = $tree""".stripMargin
         })
       tree

tests/pos/i7159.scala

@@ -0,0 +1,15 @@
+trait CompilerInterface {
+  type Tree22
+}
+
+class Reflect(val internal: CompilerInterface) {
+  opaque type Tree22 = internal.Tree22
+  def show(t: Tree22): String = ???
+}
+
+object App {
+  val refl: Reflect = ???
+  import refl._
+
+  show(??? : Tree22)
+}