Skip to content

Commit

Permalink
Avoid forcing ctors & parents which caused cycles (#17086)
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand authored Aug 11, 2024
2 parents ba2c731 + 987235e commit c04e247
Show file tree
Hide file tree
Showing 21 changed files with 248 additions and 74 deletions.
19 changes: 18 additions & 1 deletion compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,24 @@ object desugar {
override def ensureCompletions(using Context): Unit = {
def completeConstructor(sym: Symbol) =
sym.infoOrCompleter match {
case completer: Namer#ClassCompleter =>
case completer: Namer#ClassCompleter if !sym.isCompleting =>
// An example, derived from tests/run/t6385.scala
//
// class Test():
// def t1: Foo = Foo(1)
// final case class Foo(value: Int)
//
// Here's the sequence of events:
// * The symbol for Foo.apply is forced to complete
// * The symbol for the `value` parameter of the apply method is forced to complete
// * Completing that value parameter requires typing its type, which is a DerivedTypeTrees,
// which only types if it has an OriginalSymbol.
// * So if the case class hasn't been completed, we need (at least) its constructor to be completed
//
// Test tests/neg/i9294.scala is an example of why isCompleting is necessary.
// Annotations are added while completing the constructor,
// so the back reference to foo reaches here which re-initiates the constructor completion.
// So we just skip, as completion is already being triggered.
completer.completeConstructor(sym)
case _ =>
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/NamerOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ object NamerOps:
* where
*
* <context-bound-companion> is the CBCompanion type created in Definitions
* withnessRefK is a refence to the K'th witness.
* withnessRefK is a reference to the K'th witness.
*
* The companion has the same access flags as the original type.
*/
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeApplications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,9 @@ class TypeApplications(val self: Type) extends AnyVal {
*/
def hkResult(using Context): Type = self.dealias match {
case self: TypeRef =>
if (self.symbol == defn.AnyKindClass) self else self.info.hkResult
if self.symbol == defn.AnyKindClass then self
else if self.symbol.isClass then NoType // avoid forcing symbol if it's a class, not an alias to a HK type lambda
else self.info.hkResult
case self: AppliedType =>
if (self.tycon.typeSymbol.isClass) NoType else self.superType.hkResult
case self: HKTypeLambda => self.resultType
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ object Types extends TypeUtils {
*/
def isRef(sym: Symbol, skipRefined: Boolean = true)(using Context): Boolean = this match {
case this1: TypeRef =>
this1.info match { // see comment in Namer#TypeDefCompleter#typeSig
// avoid forcing symbol if it's a class, not a type alias (see i15177.FakeEnum.scala)
if this1.symbol.isClass then this1.symbol eq sym
else this1.info match { // see comment in Namer#TypeDefCompleter#typeSig
case TypeAlias(tp) => tp.isRef(sym, skipRefined)
case _ => this1.symbol eq sym
}
Expand Down
170 changes: 106 additions & 64 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,11 @@ class Namer { typer: Typer =>
if (sym.is(Module)) moduleValSig(sym)
else valOrDefDefSig(original, sym, Nil, identity)(using localContext(sym).setNewScope)
case original: DefDef =>
val typer1 = ctx.typer.newLikeThis(ctx.nestingLevel + 1)
nestedTyper(sym) = typer1
// For the primary constructor DefDef, it is:
// * indexed as a part of completing the class, with indexConstructor; and
// * typed ahead when completing the constructor
// So we need to make sure to reuse the same local/nested typer.
val typer1 = nestedTyper.getOrElseUpdate(sym, ctx.typer.newLikeThis(ctx.nestingLevel + 1))
typer1.defDefSig(original, sym, this)(using localContext(sym).setTyper(typer1))
case imp: Import =>
try
Expand All @@ -833,6 +836,12 @@ class Namer { typer: Typer =>
typr.println(s"error while completing ${imp.expr}")
throw ex

/** Context setup for indexing the constructor. */
def indexConstructor(constr: DefDef, sym: Symbol): Unit =
val typer1 = ctx.typer.newLikeThis(ctx.nestingLevel + 1)
nestedTyper(sym) = typer1
typer1.indexConstructor(constr, sym)(using localContext(sym).setTyper(typer1))

final override def complete(denot: SymDenotation)(using Context): Unit = {
if (Config.showCompletions && ctx.typerState != creationContext.typerState) {
def levels(c: Context): Int =
Expand Down Expand Up @@ -993,15 +1002,19 @@ class Namer { typer: Typer =>

/** If completion of the owner of the to be completed symbol has not yet started,
* complete the owner first and check again. This prevents cyclic references
* where we need to copmplete a type parameter that has an owner that is not
* where we need to complete a type parameter that has an owner that is not
* yet completed. Test case is pos/i10967.scala.
*/
override def needsCompletion(symd: SymDenotation)(using Context): Boolean =
val owner = symd.owner
!owner.exists
|| owner.is(Touched)
|| {
owner.ensureCompleted()
// Only complete the owner if it's a type (eg. the class that owns a type parameter)
// This avoids completing primary constructor methods while completing the type of one of its type parameters
// See i15177.scala.
if owner.isType then
owner.ensureCompleted()
!symd.isCompleted
}

Expand Down Expand Up @@ -1526,12 +1539,9 @@ class Namer { typer: Typer =>
index(constr)
index(rest)(using localCtx)

symbolOfTree(constr).info.stripPoly match // Completes constr symbol as a side effect
case mt: MethodType if cls.is(Case) && mt.isParamDependent =>
// See issue #8073 for background
report.error(
em"""Implementation restriction: case classes cannot have dependencies between parameters""",
cls.srcPos)
val constrSym = symbolOfTree(constr)
constrSym.infoOrCompleter match
case completer: Completer => completer.indexConstructor(constr, constrSym)
case _ =>

tempInfo = denot.asClass.classInfo.integrateOpaqueMembers.asInstanceOf[TempClassInfo]
Expand Down Expand Up @@ -1762,6 +1772,17 @@ class Namer { typer: Typer =>
val sym = tree.symbol
if sym.isConstructor then sym.owner else sym

/** Index the primary constructor of a class, as a part of completing that class.
* This allows the rest of the constructor completion to be deferred,
* which avoids non-cyclic classes failing, e.g. pos/i15177.
*/
def indexConstructor(constr: DefDef, sym: Symbol)(using Context): Unit =
index(constr.leadingTypeParams)
sym.owner.typeParams.foreach(_.ensureCompleted())
completeTrailingParamss(constr, sym, indexingCtor = true)
if Feature.enabled(modularity) then
constr.termParamss.foreach(_.foreach(setTracked))

/** The signature of a module valdef.
* This will compute the corresponding module class TypeRef immediately
* without going through the defined type of the ValDef. This is necessary
Expand Down Expand Up @@ -1860,31 +1881,6 @@ class Namer { typer: Typer =>
// Beware: ddef.name need not match sym.name if sym was freshened!
val isConstructor = sym.name == nme.CONSTRUCTOR

// A map from context-bounded type parameters to associated evidence parameter names
val witnessNamesOfParam = mutable.Map[TypeDef, List[TermName]]()
if !ddef.name.is(DefaultGetterName) && !sym.is(Synthetic) then
for params <- ddef.paramss; case tdef: TypeDef <- params do
for case WitnessNamesAnnot(ws) <- tdef.mods.annotations do
witnessNamesOfParam(tdef) = ws

/** Is each name in `wnames` defined somewhere in the longest prefix of all `params`
* that have been typed ahead (i.e. that carry the TypedAhead attachment)?
*/
def allParamsSeen(wnames: List[TermName], params: List[MemberDef]) =
(wnames.toSet[Name] -- params.takeWhile(_.hasAttachment(TypedAhead)).map(_.name)).isEmpty

/** Enter and typecheck parameter list.
* Once all witness parameters for a context bound are seen, create a
* context bound companion for it.
*/
def completeParams(params: List[MemberDef])(using Context): Unit =
index(params)
for param <- params do
typedAheadExpr(param)
for (tdef, wnames) <- witnessNamesOfParam do
if wnames.contains(param.name) && allParamsSeen(wnames, params) then
addContextBoundCompanionFor(symbolOfTree(tdef), wnames, params.map(symbolOfTree))

// The following 3 lines replace what was previously just completeParams(tparams).
// But that can cause bad bounds being computed, as witnessed by
// tests/pos/paramcycle.scala. The problematic sequence is this:
Expand All @@ -1908,39 +1904,16 @@ class Namer { typer: Typer =>
// 3. Info of CP is computed (to be copied to DP).
// 4. CP is completed.
// 5. Info of CP is copied to DP and DP is completed.
index(ddef.leadingTypeParams)
if (isConstructor) sym.owner.typeParams.foreach(_.ensureCompleted())
if !sym.isPrimaryConstructor then
index(ddef.leadingTypeParams)
val completedTypeParams =
for tparam <- ddef.leadingTypeParams yield typedAheadExpr(tparam).symbol
if completedTypeParams.forall(_.isType) then
completer.setCompletedTypeParams(completedTypeParams.asInstanceOf[List[TypeSymbol]])
ddef.trailingParamss.foreach(completeParams)
completeTrailingParamss(ddef, sym, indexingCtor = false)
val paramSymss = normalizeIfConstructor(ddef.paramss.nestedMap(symbolOfTree), isConstructor)
sym.setParamss(paramSymss)

/** Under x.modularity, we add `tracked` to context bound witnesses
* that have abstract type members
*/
def needsTracked(sym: Symbol, param: ValDef)(using Context) =
!sym.is(Tracked)
&& param.hasAttachment(ContextBoundParam)
&& sym.info.memberNames(abstractTypeNameFilter).nonEmpty

/** Under x.modularity, set every context bound evidence parameter of a class to be tracked,
* provided it has a type that has an abstract type member. Reset private and local flags
* so that the parameter becomes a `val`.
*/
def setTracked(param: ValDef): Unit =
val sym = symbolOfTree(param)
sym.maybeOwner.maybeOwner.infoOrCompleter match
case info: TempClassInfo if needsTracked(sym, param) =>
typr.println(i"set tracked $param, $sym: ${sym.info} containing ${sym.info.memberNames(abstractTypeNameFilter).toList}")
for acc <- info.decls.lookupAll(sym.name) if acc.is(ParamAccessor) do
acc.resetFlag(PrivateLocal)
acc.setFlag(Tracked)
sym.setFlag(Tracked)
case _ =>

def wrapMethType(restpe: Type): Type =
instantiateDependent(restpe, paramSymss)
methodType(paramSymss, restpe, ddef.mods.is(JavaDefined))
Expand All @@ -1949,11 +1922,11 @@ class Namer { typer: Typer =>
wrapMethType(addParamRefinements(restpe, paramSymss))

if isConstructor then
if sym.isPrimaryConstructor && Feature.enabled(modularity) then
ddef.termParamss.foreach(_.foreach(setTracked))
// set result type tree to unit, but take the current class as result type of the symbol
typedAheadType(ddef.tpt, defn.UnitType)
wrapMethType(effectiveResultType(sym, paramSymss))
val mt = wrapMethType(effectiveResultType(sym, paramSymss))
if sym.isPrimaryConstructor then checkCaseClassParamDependencies(mt, sym.owner)
mt
else if sym.isAllOf(Given | Method) && Feature.enabled(modularity) then
// set every context bound evidence parameter of a given companion method
// to be tracked, provided it has a type that has an abstract type member.
Expand All @@ -1966,6 +1939,75 @@ class Namer { typer: Typer =>
valOrDefDefSig(ddef, sym, paramSymss, wrapMethType)
end defDefSig

/** Complete the trailing parameters of a DefDef,
* as a part of indexing the primary constructor or
* as a part of completing a DefDef, including the primary constructor.
*/
def completeTrailingParamss(ddef: DefDef, sym: Symbol, indexingCtor: Boolean)(using Context): Unit =
// A map from context-bounded type parameters to associated evidence parameter names
val witnessNamesOfParam = mutable.Map[TypeDef, List[TermName]]()
if !ddef.name.is(DefaultGetterName) && !sym.is(Synthetic) && (indexingCtor || !sym.isPrimaryConstructor) then
for params <- ddef.paramss; case tdef: TypeDef <- params do
for case WitnessNamesAnnot(ws) <- tdef.mods.annotations do
witnessNamesOfParam(tdef) = ws

/** Is each name in `wnames` defined somewhere in the previous parameters? */
def allParamsSeen(wnames: List[TermName], prevParams: Set[Name]) =
(wnames.toSet[Name] -- prevParams).isEmpty

/** Enter and typecheck parameter list.
* Once all witness parameters for a context bound are seen, create a
* context bound companion for it.
*/
def completeParams(params: List[MemberDef])(using Context): Unit =
if indexingCtor || !sym.isPrimaryConstructor then
index(params)
var prevParams = Set.empty[Name]
for param <- params do
if !indexingCtor then
typedAheadExpr(param)

prevParams += param.name
for (tdef, wnames) <- witnessNamesOfParam do
if wnames.contains(param.name) && allParamsSeen(wnames, prevParams) then
addContextBoundCompanionFor(symbolOfTree(tdef), wnames, params.map(symbolOfTree))

ddef.trailingParamss.foreach(completeParams)
end completeTrailingParamss

/** Checks an implementation restriction on case classes. */
def checkCaseClassParamDependencies(mt: Type, cls: Symbol)(using Context): Unit =
mt.stripPoly match
case mt: MethodType if cls.is(Case) && mt.isParamDependent =>
// See issue #8073 for background
report.error(
em"""Implementation restriction: case classes cannot have dependencies between parameters""",
cls.srcPos)
case _ =>

/** Under x.modularity, we add `tracked` to context bound witnesses
* that have abstract type members
*/
def needsTracked(sym: Symbol, param: ValDef)(using Context) =
!sym.is(Tracked)
&& param.hasAttachment(ContextBoundParam)
&& sym.info.memberNames(abstractTypeNameFilter).nonEmpty

/** Under x.modularity, set every context bound evidence parameter of a class to be tracked,
* provided it has a type that has an abstract type member. Reset private and local flags
* so that the parameter becomes a `val`.
*/
def setTracked(param: ValDef)(using Context): Unit =
val sym = symbolOfTree(param)
sym.maybeOwner.maybeOwner.infoOrCompleter match
case info: ClassInfo if needsTracked(sym, param) =>
typr.println(i"set tracked $param, $sym: ${sym.info} containing ${sym.info.memberNames(abstractTypeNameFilter).toList}")
for acc <- info.decls.lookupAll(sym.name) if acc.is(ParamAccessor) do
acc.resetFlag(PrivateLocal)
acc.setFlag(Tracked)
sym.setFlag(Tracked)
case _ =>

def inferredResultType(
mdef: ValOrDefDef,
sym: Symbol,
Expand Down
6 changes: 0 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2516,12 +2516,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
(arg, tparamBounds)
else
(arg, WildcardType)
if (tpt1.symbol.isClass)
tparam match {
case tparam: Symbol =>
tparam.ensureCompleted() // This is needed to get the test `compileParSetSubset` to work
case _ =>
}
if (desugaredArg.isType)
arg match {
case untpd.WildcardTypeBoundsTree()
Expand Down
7 changes: 7 additions & 0 deletions tests/neg/i15177.FakeEnum.min.alt1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Like tests/neg/i15177.FakeEnum.min.scala
// But with an actual upper-bound requirement
// Which shouldn't be ignored as a part of overcoming the the cycle
trait Foo
trait X[T <: Foo] { trait Id }
object A extends X[B] // error: Type argument B does not conform to upper bound Foo
class B extends A.Id
9 changes: 9 additions & 0 deletions tests/neg/i15177.constr-dep.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// An example of how constructor _type_ parameters
// Which can _not_ be passed to the extends part
// That makes it part of the parent type,
// which has been found to be unsound.
class Foo[A]
class Foo1(val x: Int)
extends Foo[ // error: The type of a class parent cannot refer to constructor parameters, but Foo[(Foo1.this.x : Int)] refers to x
x.type
]
13 changes: 13 additions & 0 deletions tests/neg/i15177.ub.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// like tests/pos/i15177.scala
// but with T having an upper bound
// that B doesn't conform to
// just to be sure that not forcing B
// doesn't backdoor an illegal X[B]
class X[T <: C] {
type Id
}
object A
extends X[ // error
B] // error
class B(id: A.Id)
class C
7 changes: 7 additions & 0 deletions tests/pos/i15177.FakeEnum.min.alt2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Like tests/neg/i15177.FakeEnum.min.scala
// With an actual upper-bound requirement
// But that is satisfied on class B
trait Foo
trait X[T <: Foo] { trait Id }
object A extends X[B]
class B extends A.Id with Foo
7 changes: 7 additions & 0 deletions tests/pos/i15177.FakeEnum.min.alt3.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Like tests/neg/i15177.FakeEnum.min.scala
// With an actual upper-bound requirement
// But that is satisfied on trait Id
trait Foo
trait X[T <: Foo] { trait Id extends Foo }
object A extends X[B]
class B extends A.Id
4 changes: 4 additions & 0 deletions tests/pos/i15177.FakeEnum.min.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Minimisation of tests/neg/i15177.FakeEnum.scala
trait X[T] { trait Id }
object A extends X[B]
class B extends A.Id
21 changes: 21 additions & 0 deletions tests/pos/i15177.FakeEnum.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// From https://github.com/scala/scala3/issues/15177#issuecomment-1463088400
trait FakeEnum[A, @specialized(Byte, Short, Int, Long) B]
{
trait Value {
self: A =>
def name: String
def id: B
}
}

object FakeEnumType
extends FakeEnum[FakeEnumType, Short]
{
val MEMBER1 = new FakeEnumType((0: Short), "MEMBER1") {}
val MEMBER2 = new FakeEnumType((1: Short), "MEMBER2") {}
}

sealed abstract
class FakeEnumType(val id: Short, val name: String)
extends FakeEnumType.Value
{}
Loading

0 comments on commit c04e247

Please sign in to comment.