Skip to content

Fix #7159: Add missing asSeenFrom when updating denotations #7166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 45 additions & 15 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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))
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 3 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/ElimOpaque.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Erasure.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions tests/pos/i7159.scala
Original file line number Diff line number Diff line change
@@ -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)
}