Skip to content
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

Avoid forcing ctors & parents which caused cycles #17086

Merged
merged 2 commits into from
Aug 11, 2024
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
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
dwijnand marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading