Skip to content

Backport "Avoid forcing ctors & parents which caused cycles" to LTS #22093

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

Closed
wants to merge 2 commits into from
Closed
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
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 @@ -85,7 +85,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
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 @@ -256,7 +256,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 @@ -195,7 +195,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
86 changes: 67 additions & 19 deletions compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -800,8 +800,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 @@ -811,6 +814,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 @@ -966,15 +975,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 @@ -1498,12 +1511,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 @@ -1711,11 +1721,14 @@ class Namer { typer: Typer =>
val sym = tree.symbol
if sym.isConstructor then sym.owner else sym

/** Enter and typecheck parameter list */
def completeParams(params: List[MemberDef])(using Context): Unit = {
index(params)
for (param <- params) typedAheadExpr(param)
}
/** 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)

/** The signature of a module valdef.
* This will compute the corresponding module class TypeRef immediately
Expand Down Expand Up @@ -1819,26 +1832,61 @@ 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)

def wrapMethType(restpe: Type): Type =
instantiateDependent(restpe, paramSymss)
methodType(paramSymss, restpe, ddef.mods.is(JavaDefined))
if isConstructor then
// 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
valOrDefDefSig(ddef, sym, paramSymss, wrapMethType)
}

/** 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 =
/** 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

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 _ =>

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 @@ -2221,12 +2221,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
{}
6 changes: 6 additions & 0 deletions tests/pos/i15177.app.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// like tests/pos/i15177.scala
// but with an applied type B[D]
class X[T] { type Id }
object A extends X[B[D]]
class B[C](id: A.Id)
class D
6 changes: 6 additions & 0 deletions tests/pos/i15177.constr-dep.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// An example of how constructor _term_ parameters
// Can be passed to the extends part
// But that doesn't mean the parent type,
// it's just the super constructor call.
class Bar(val y: Long)
class Bar1(val z: Long) extends Bar(z)
5 changes: 5 additions & 0 deletions tests/pos/i15177.hk.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// like tests/pos/i15177.scala
// but with B being higher kinded
class X[T[_]] { type Id }
object A extends X[B]
class B[C](id: A.Id)
6 changes: 6 additions & 0 deletions tests/pos/i15177.hk2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// like tests/pos/i15177.scala
// but with B being higher kinded
// but without the actual cycle (like .without)
class X[T[_]] { type Id }
class A extends X[B]
class B[C]
7 changes: 7 additions & 0 deletions tests/pos/i15177.orig.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
trait DomainIdProvider[T] {
type Id = List[T]
}
object Country extends DomainIdProvider[Country]
case class Country(
id: Country.Id,
)
3 changes: 3 additions & 0 deletions tests/pos/i15177.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class X[T] { trait Id }
object A extends X[B]
class B(id: A.Id)
5 changes: 5 additions & 0 deletions tests/pos/i15177.without.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// like tests/pos/i15177.scala
// but without the actual cycle
class X[T] { trait Id }
class A extends X[B]
class B
Loading