Skip to content

Fix #8152: Rearchitect some parts of opaque types handling #8170

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 12 commits into from
Feb 5, 2020
Merged
26 changes: 2 additions & 24 deletions compiler/src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1165,30 +1165,8 @@ object Denotations {
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 == 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`
// and this is the first access
// 2. `T` will normalize to an abstract type on completion.
// 3. At that time, the default logic in the second case is wrong: `T`'s new info
// is now an abstract type and running it through an asSeenFrom gives nothing.
// We fix this as follows:
// 1. Force opaque normalization as first step
// 2. Read the info from the enclosing object's refinement
symbol.normalizeOpaque()
def findRefined(tp: Type, name: Name): Type = tp match {
case RefinedType(parent, rname, rinfo) =>
if (rname == name) rinfo else findRefined(parent, name)
case _ =>
symbol.info
}
derived(findRefined(pre.underlying, symbol.name))
case _ =>
if (!owner.membersNeedAsSeenFrom(pre) || symbol.is(NonMember)) this
else derived(symbol.info)
}
if (!owner.membersNeedAsSeenFrom(pre) || symbol.is(NonMember)) this
else derived(symbol.info)
}

/** Does this denotation have all the `required` flags but none of the `excluded` flags?
Expand Down
72 changes: 37 additions & 35 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -426,42 +426,44 @@ object SymDenotations {
case _ => unforcedDecls.openForMutations
}

/** If this is a synthetic opaque type alias, mark it as Deferred with bounds
* as given by the right hand side's `WithBounds` annotation, if one is present,
* or with empty bounds of the right kind, otherwise.
* At the same time, integrate the original alias as a refinement of the
/** If this is an opaque alias, replace the right hand side `info`
* by appropriate bounds and store `info` in the refinement of the
* self type of the enclosing class.
* Otherwise return `info`
*
* @param info Is assumed to be a (lambda-abstracted) right hand side TypeAlias
* of the opaque type definition.
*/
final def normalizeOpaque()(implicit ctx: Context) = {
def abstractRHS(tp: Type): Type = tp match {
case tp: HKTypeLambda => tp.derivedLambdaType(resType = abstractRHS(tp.resType))
case _ => defn.AnyType
}
if (isOpaqueAlias)
info match {
case TypeAlias(alias) =>
val (refiningAlias, bounds) = alias match {
case AnnotatedType(alias1, Annotation.WithBounds(bounds)) =>
(alias1, bounds)
case _ =>
(alias, TypeBounds(defn.NothingType, abstractRHS(alias)))
}
def refineSelfType(selfType: Type) =
RefinedType(selfType, name, TypeAlias(refiningAlias))
val enclClassInfo = owner.asClass.classInfo
enclClassInfo.selfInfo match {
case self: Type =>
owner.info = enclClassInfo.derivedClassInfo(
selfInfo = refineSelfType(self.orElse(defn.AnyType)))
case self: Symbol =>
self.info = refineSelfType(self.info)
}
info = bounds
setFlag(Deferred)
typeRef.recomputeDenot()
case _ =>
}
}
def opaqueToBounds(info: Type)(given Context): Type =

def setAlias(tp: Type) =
def recur(self: Type): Unit = self match
case RefinedType(parent, name, rinfo) => rinfo match
case TypeAlias(lzy: LazyRef) if name == this.name =>
lzy.update(tp)
case _ =>
recur(parent)
recur(owner.asClass.givenSelfType)
end setAlias

def split(tp: Type): (Type, TypeBounds) = tp match
case AnnotatedType(alias, Annotation.WithBounds(bounds)) =>
(alias, bounds)
case tp: HKTypeLambda =>
val (alias1, bounds1) = split(tp.resType)
(tp.derivedLambdaType(resType = alias1),
HKTypeLambda.boundsFromParams(tp.typeParams, bounds1))
case _ =>
(tp, HKTypeLambda.boundsFromParams(tp.typeParams, TypeBounds.empty))

info match
case TypeAlias(tp) if isOpaqueAlias && owner.isClass =>
val (alias, bounds) = split(tp)
setAlias(alias)
bounds
case _ =>
info
end opaqueToBounds

// ------ Names ----------------------------------------------

Expand Down Expand Up @@ -1203,7 +1205,7 @@ object SymDenotations {
def opaqueAlias(implicit ctx: Context): Type = {
def recur(tp: Type): Type = tp match {
case RefinedType(parent, rname, TypeAlias(alias)) =>
if (rname == name) alias else recur(parent)
if rname == name then alias.stripLazyRef else recur(parent)
case _ =>
NoType
}
Expand Down
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1727,10 +1727,14 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || tp2.isRef(NothingClass) then tp2
else if tp2.isAny && !tp1.isLambdaSub || tp2.isAnyKind || tp1.isRef(NothingClass) then tp1
else tp2 match { // normalize to disjunctive normal form if possible.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use tp2.stripLazyRef instead ? (same thing below and in lub)

case tp2: LazyRef =>
glb(tp1, tp2.ref)
case OrType(tp21, tp22) =>
tp1 & tp21 | tp1 & tp22
case _ =>
tp1 match {
case tp1: LazyRef =>
glb(tp1.ref, tp2)
case OrType(tp11, tp12) =>
tp11 & tp2 | tp12 & tp2
case _ =>
Expand Down Expand Up @@ -1777,8 +1781,8 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
else if (!tp2.exists) tp2
else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || tp2.isRef(NothingClass) then tp1
else if tp2.isAny && !tp1.isLambdaSub || tp2.isAnyKind || tp1.isRef(NothingClass) then tp2
else {
def mergedLub: Type = {
else
def mergedLub(tp1: Type, tp2: Type): Type = {
val atoms1 = tp1.atoms(widenOK = true)
if (atoms1.nonEmpty && !widenInUnions) {
val atoms2 = tp2.atoms(widenOK = true)
Expand All @@ -1800,8 +1804,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
if ((tp1 ne tp1w) || (tp2 ne tp2w)) lub(tp1w, tp2w)
else orType(tp1w, tp2w) // no need to check subtypes again
}
mergedLub
}
mergedLub(tp1.stripLazyRef, tp2.stripLazyRef)
}

/** The least upper bound of a list of types */
Expand Down
92 changes: 70 additions & 22 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,8 @@ object Types {
* pos/i536 demonstrates that the infinite loop can also involve lower bounds.
*/
def safe_& (that: Type)(implicit ctx: Context): Type = (this, that) match {
case (TypeBounds(lo1, hi1), TypeBounds(lo2, hi2)) => TypeBounds(OrType(lo1, lo2), AndType(hi1, hi2))
case (TypeBounds(lo1, hi1), TypeBounds(lo2, hi2)) =>
TypeBounds(OrType(lo1.stripLazyRef, lo2.stripLazyRef), AndType(hi1.stripLazyRef, hi2.stripLazyRef))
case _ => this & that
}

Expand Down Expand Up @@ -1043,12 +1044,17 @@ object Types {
case _ => this
}

/** Strip PolyType prefix */
/** Strip PolyType prefixes */
def stripPoly(implicit ctx: Context): Type = this match {
case tp: PolyType => tp.resType.stripPoly
case _ => this
}

/** Strip LazyRef wrappers */
def stripLazyRef(given Context): Type = this match
case lzy: LazyRef => lzy.ref.stripLazyRef
case _ => this

/** Widen from singleton type to its underlying non-singleton
* base type by applying one or more `underlying` dereferences,
* Also go from => T to T.
Expand Down Expand Up @@ -1165,7 +1171,7 @@ object Types {
* these types as a set, otherwise the empty set.
* Overridden and cached in OrType.
* @param widenOK If type proxies that are upperbounded by types with atoms
* have the same atoms.
* have the same atoms.
*/
def atoms(widenOK: Boolean = false)(implicit ctx: Context): Set[Type] = dealias match {
case tp: SingletonType =>
Expand Down Expand Up @@ -2566,25 +2572,34 @@ object Types {
}
}

case class LazyRef(private var refFn: Context => Type) extends UncachedProxyType with ValueType {
case class LazyRef(private var refFn: Context => Type, reportCycles: Boolean = false) extends UncachedProxyType with ValueType {
private var myRef: Type = null
private var computed = false
def ref(implicit ctx: Context): Type = {
if (computed) {
if (myRef == null) {

def ref(implicit ctx: Context): Type =
if computed then
if myRef == null then
// if errors were reported previously handle this by throwing a CyclicReference
// instead of crashing immediately. A test case is neg/i6057.scala.
assert(ctx.reporter.errorsReported)
assert(reportCycles || ctx.reporter.errorsReported)
throw CyclicReference(NoDenotation)
}
}
else {
else
computed = true
myRef = refFn(ctx)
val result = refFn(ctx)
refFn = null
}
if result != null then myRef = result
else assert(myRef != null) // must have been `update`d
myRef
}

/** Update the value of the lazyref, discarding the compute function `refFn`
* Can be called only as long as the ref is still undefined.
*/
def update(tp: Type) =
assert(myRef == null)
myRef = tp
computed = true
refFn = null

def evaluating: Boolean = computed && myRef == null
def completed: Boolean = myRef != null
override def underlying(implicit ctx: Context): Type = ref
Expand Down Expand Up @@ -4300,13 +4315,47 @@ object Types {
parentsCache
}

protected def newLikeThis(prefix: Type, classParents: List[Type], decls: Scope, selfInfo: TypeOrSymbol)(given Context): ClassInfo =
ClassInfo(prefix, cls, classParents, decls, selfInfo)

def derivedClassInfo(prefix: Type)(implicit ctx: Context): ClassInfo =
if (prefix eq this.prefix) this
else ClassInfo(prefix, cls, classParents, decls, selfInfo)
else newLikeThis(prefix, classParents, decls, selfInfo)

def derivedClassInfo(prefix: Type = this.prefix, classParents: List[Type] = this.classParents, decls: Scope = this.decls, selfInfo: TypeOrSymbol = this.selfInfo)(implicit ctx: Context): ClassInfo =
if ((prefix eq this.prefix) && (classParents eq this.classParents) && (decls eq this.decls) && (selfInfo eq this.selfInfo)) this
else ClassInfo(prefix, cls, classParents, decls, selfInfo)
else newLikeThis(prefix, classParents, decls, selfInfo)

/** If this class has opaque type alias members, a new class info
* with their aliases added as refinements to the self type of the class.
* Otherwise, this classInfo.
* If there are opaque alias members, updates `cls` to have `Opaque` flag as a side effect.
*/
def integrateOpaqueMembers(given Context): ClassInfo =
decls.toList.foldLeft(this) { (cinfo, sym) =>
if sym.isOpaqueAlias then
cls.setFlag(Opaque)
def force =
if sym.isOpaqueAlias then // could have been reset because of a syntax error
sym.infoOrCompleter match
case completer: LazyType =>
completer.complete(sym) // will update the LazyRef
null // tells the LazyRef to use the updated value
case info => // can occur under cyclic references, e.g. i6225.scala
defn.AnyType
else defn.AnyType // dummy type in case of errors
def refineSelfType(selfType: Type) =
RefinedType(selfType, sym.name,
TypeAlias(LazyRef(_ => force, reportCycles = true)))
cinfo.selfInfo match
case self: Type =>
cinfo.derivedClassInfo(
selfInfo = refineSelfType(self.orElse(defn.AnyType)))
case self: Symbol =>
self.info = refineSelfType(self.info)
cinfo
else cinfo
}

override def computeHash(bs: Binders): Int = doHash(bs, cls, prefix)
override def hashIsStable: Boolean = prefix.hashIsStable && classParents.hashIsStable
Expand Down Expand Up @@ -4343,13 +4392,12 @@ object Types {
final class TempClassInfo(prefix: Type, cls: ClassSymbol, decls: Scope, selfInfo: TypeOrSymbol)
extends CachedClassInfo(prefix, cls, Nil, decls, selfInfo) {

/** Install classinfo with known parents in `denot` s */
def finalize(denot: SymDenotation, parents: List[Type])(implicit ctx: Context): Unit =
denot.info = ClassInfo(prefix, cls, parents, decls, selfInfo)
/** Convert to classinfo with known parents */
def finalized(parents: List[Type])(implicit ctx: Context): ClassInfo =
ClassInfo(prefix, cls, parents, decls, selfInfo)

override def derivedClassInfo(prefix: Type)(implicit ctx: Context): ClassInfo =
if (prefix eq this.prefix) this
else new TempClassInfo(prefix, cls, decls, selfInfo)
override def newLikeThis(prefix: Type, classParents: List[Type], decls: Scope, selfInfo: TypeOrSymbol)(given Context): ClassInfo =
TempClassInfo(prefix, cls, decls, selfInfo)

override def toString: String = s"TempClassInfo($prefix, $cls)"
}
Expand Down
16 changes: 10 additions & 6 deletions compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@ class TreeUnpickler(reader: TastyReader,
ctx.newSymbol(ctx.owner, name, flags, completer, privateWithin, coord)
}
sym.annotations = annotFns.map(_(sym))
if sym.isOpaqueAlias then sym.setFlag(Deferred)
ctx.owner match {
case cls: ClassSymbol => cls.enter(sym)
case _ =>
Expand Down Expand Up @@ -832,11 +833,12 @@ class TreeUnpickler(reader: TastyReader,
override def completerTypeParams(sym: Symbol)(implicit ctx: Context) =
rhs.tpe.typeParams
}
sym.info = rhs.tpe match {
case _: TypeBounds | _: ClassInfo => checkNonCyclic(sym, rhs.tpe, reportErrors = false)
case _ => rhs.tpe.toBounds
sym.info = sym.opaqueToBounds {
rhs.tpe match
case _: TypeBounds | _: ClassInfo => checkNonCyclic(sym, rhs.tpe, reportErrors = false)
case _ => rhs.tpe.toBounds
}
sym.normalizeOpaque()
if sym.isOpaqueAlias then sym.typeRef.recomputeDenot() // make sure we see the new bounds from now on
sym.resetFlag(Provisional)
TypeDef(rhs)
}
Expand Down Expand Up @@ -905,8 +907,10 @@ class TreeUnpickler(reader: TastyReader,
}
else EmptyValDef
cls.setNoInitsFlags(parentsKind(parents), bodyFlags)
cls.info = ClassInfo(cls.owner.thisType, cls, parentTypes, cls.unforcedDecls,
if (self.isEmpty) NoType else self.tpt.tpe)
cls.info = ClassInfo(
cls.owner.thisType, cls, parentTypes, cls.unforcedDecls,
selfInfo = if (self.isEmpty) NoType else self.tpt.tpe)
.integrateOpaqueMembers
val constr = readIndexedDef().asInstanceOf[DefDef]
val mappedParents = parents.map(_.changeOwner(localDummy, constr.symbol))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ object Scala2Unpickler {
else
registerCompanionPair(scalacCompanion, denot.classSymbol)

tempInfo.finalize(denot, normalizedParents) // install final info, except possibly for typeparams ordering
denot.info = tempInfo.finalized(normalizedParents)
denot.ensureTypeParamsInCorrectOrder()
}
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,11 @@ class PlainPrinter(_ctx: Context) extends Printer {
}
case tp: ExprType =>
changePrec(GlobalPrec) { "=> " ~ toText(tp.resultType) }
case tp: TypeLambda =>
case tp: HKTypeLambda =>
changePrec(GlobalPrec) {
"[" ~ paramsText(tp) ~ "]" ~ lambdaHash(tp) ~ Str(" =>> ") ~ toTextGlobal(tp.resultType)
}
case tp: PolyType =>
changePrec(GlobalPrec) {
"[" ~ paramsText(tp) ~ "]" ~ lambdaHash(tp) ~
(Str(" => ") provided !tp.resultType.isInstanceOf[MethodType]) ~
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1262,7 +1262,8 @@ object messages {
case class CyclicReferenceInvolving(denot: SymDenotation)(implicit ctx: Context)
extends Message(CyclicReferenceInvolvingID) {
val kind: String = "Cyclic"
val msg: String = em"""Cyclic reference involving $denot"""
val where = if denot.exists then s" involving $denot" else ""
val msg: String = em"Cyclic reference $where"
val explanation: String =
em"""|$denot is declared as part of a cycle which makes it impossible for the
|compiler to decide upon ${denot.name}'s type.
Expand Down
Loading