Skip to content

Init check: Early Promotion of fields #11533

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 13 commits into from
Apr 29, 2021
146 changes: 108 additions & 38 deletions compiler/src/dotty/tools/dotc/transform/init/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -264,55 +264,125 @@ object Checking {
val Summary(pots, effs) = expand(pot)
val effs2 = pots.map(FieldAccess(_, field)(eff.source))
(effs2 ++ effs).toList.flatMap(check(_))
}

/**
* Check if we can just directly promote a potential.
* A potential can be (currently) directly promoted if and only if:
* - `pot == this` and all fields of this are initialized, or
* - `pot == Warm(C, outer)` where `outer` can be directly promoted.
*/
private def canDirectlyPromote(pot: Potential, visited: Set[Potential] = Set.empty)(using state: State): Boolean = trace("checking direct promotion of " + pot.show, init) {
if (state.safePromoted.contains(pot)) true
// If this potential's promotion depends on itself, we cannot directly promote it.
else if (visited.contains(pot)) false
else pot match {
case pot: ThisRef =>
// If we have all fields initialized, then we can promote This to hot.
val classRef = state.thisClass.info.asInstanceOf[ClassInfo].appliedRef
classRef.fields.forall { denot =>
val sym = denot.symbol
sym.isOneOf(Flags.Lazy | Flags.Deferred) || state.fieldsInited.contains(sym)
}
case Warm(cls, outer) =>
canDirectlyPromote(outer)
case _ =>
val summary = expand(pot)
Copy link
Contributor

@liufengyun liufengyun Feb 25, 2021

Choose a reason for hiding this comment

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

This will lead to infinite loops. The following might be a test case:

class A {
  val a = f
  def f: A = f
  class B(x: Int)

  println(new a.B(5))
  val n = 10
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mitigating this with a visited set. I think it's easier to just set a maximum recursion depth though, because this is only a speed-up measure most of the time...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you can play with it and see which works better for the tests we have.

if (!summary.effs.isEmpty)
false // max depth of expansion reached
else summary.pots.forall(canDirectlyPromote(_, visited + pot))
}
}

/**
* Check the Promotion of a Warm object, according to "Rule 2":
*
* Rule 2: Promote(pot)
*
* for all concrete methods `m` of D
* pot.m!, Promote(pot.m)
*
* for all concrete fields `f` of D
* Promote(pot.f)
*
* for all inner classes `F` of D
* Warm[F, pot].init!, Promote(Warm[F, pot])
*/
private def checkPromoteWarm(warm: Warm, eff: Effect)(using state: State): Errors =
val Warm(cls, outer) = warm
val source = eff.source
// Errors.empty
val classRef = cls.info.asInstanceOf[ClassInfo].appliedRef
// All members of class must be promotable.
val buffer = new mutable.ArrayBuffer[Effect]
val excludedFlags = Flags.Deferred | Flags.Private | Flags.Protected

classRef.fields.foreach { denot =>
val f = denot.symbol
if !f.isOneOf(excludedFlags) && f.hasSource then
buffer += Promote(FieldReturn(warm, f)(source))(source)
buffer += FieldAccess(warm, f)(source)
}

classRef.membersBasedOnFlags(Flags.Method, Flags.Deferred).foreach { denot =>
val m = denot.symbol
if !m.isConstructor && m.hasSource && !theEnv.canIgnoreMethod(m) then
buffer += MethodCall(warm, m)(source)
buffer += Promote(MethodReturn(warm, m)(source))(source)
}

classRef.memberClasses.foreach { denot =>
val cls = denot.symbol.asClass
if cls.hasSource then
val potInner = Potentials.asSeenFrom(Warm(cls, ThisRef()(source))(source), warm)
buffer += MethodCall(potInner, cls.primaryConstructor)(source)
buffer += Promote(potInner)(source)
}

for (eff <- buffer.toList) {
val errs = check(eff)
if !errs.isEmpty then
return UnsafePromotion(warm, eff.source, state.path, errs.toList).toErrors
}
Errors.empty

private def checkPromote(eff: Promote)(using state: State): Errors =
if (state.safePromoted.contains(eff.potential)) Errors.empty
else {
val pot = eff.potential
val errs = pot match {
case pot: ThisRef =>
// If we have all fields initialized, then we can promote This to hot.
val classRef = state.thisClass.info.asInstanceOf[ClassInfo].appliedRef
val allFieldsInited = classRef.fields.forall { denot =>
val sym = denot.symbol
sym.isOneOf(Flags.Lazy | Flags.Deferred) || state.fieldsInited.contains(sym)
}
if (allFieldsInited)
Errors.empty
else
PromoteThis(pot, eff.source, state.path).toErrors

case _: Cold =>
PromoteCold(eff.source, state.path).toErrors

case pot @ Warm(cls, outer) =>
val errors = state.test { checkPromote(Promote(outer)(eff.source)) }
if (errors.isEmpty) Errors.empty
else PromoteWarm(pot, eff.source, state.path).toErrors

case Fun(pots, effs) =>
val errs1 = state.test {
effs.toList.flatMap(check(_))
}
val errs2 = state.test {
pots.toList.flatMap { pot =>
checkPromote(Promote(pot)(eff.source))
val errs =
if canDirectlyPromote(pot) then
Errors.empty
else pot match {
case pot: ThisRef =>
PromoteThis(pot, eff.source, state.path).toErrors

case _: Cold =>
PromoteCold(eff.source, state.path).toErrors

case pot @ Warm(cls, outer) =>
checkPromoteWarm(pot, eff)

case Fun(pots, effs) =>
val errs1 = state.test {
effs.toList.flatMap(check(_))
}
val errs2 = state.test {
pots.toList.flatMap { pot =>
checkPromote(Promote(pot)(eff.source))
}
}
}

if (errs1.nonEmpty || errs2.nonEmpty)
UnsafePromotion(pot, eff.source, state.path, errs1 ++ errs2).toErrors
else
Errors.empty
if (errs1.nonEmpty || errs2.nonEmpty)
UnsafePromotion(pot, eff.source, state.path, errs1 ++ errs2).toErrors
else
Errors.empty

case pot =>
val Summary(pots, effs) = expand(pot)
val effs2 = pots.map(Promote(_)(eff.source))
(effs2 ++ effs).toList.flatMap(check(_))
}
case pot =>
val Summary(pots, effs) = expand(pot)
val effs2 = pots.map(Promote(_)(eff.source))
(effs2 ++ effs).toList.flatMap(check(_))
}
// If we can safely promote, then we don't need to check again
if (errs.isEmpty)
state.safePromoted += pot
Expand Down
2 changes: 1 addition & 1 deletion tests/init/neg/inner1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Foo {
val list = List(1, 2, 3) // error, as Inner access `this.list`

val inner: Inner = new this.Inner // ok, `list` is instantiated
lib.escape(inner) // error
lib.escape(inner) // ok, can promote inner early

val name = "good"

Expand Down
2 changes: 1 addition & 1 deletion tests/init/neg/inner17.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class A {
val a = f
}

println(new B) // error
println(new B) // OK, can promote B early
}

class C extends A {
Expand Down
4 changes: 2 additions & 2 deletions tests/init/neg/inner19.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ class A {

class B extends A {
println((new O.B).f)
O.C(4) // error
override val n = 50 // error
O.C(4)
override val n = 50 // error because line 16
}
38 changes: 38 additions & 0 deletions tests/init/pos/early-promote.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
class Y {
class X {
class B {
def g = f
def g2 = n
}
val f = 42
val b = new B // warm(B, X.this)
}

val n = 10
val x = new X
List(x.b) // unsafe promotion

}

class A { // checking A
class B {
def bf = 42
class C {
def x = bf // uses outer[C], but never outer[B]
}
List((new C).x)
def c = new C
}
val b = new B()
List(b) // Direct promotion works here
val af = 42
}

class RecursiveF {
val a = f
def f: RecursiveF = f
class B(x: Int)

println(new a.B(5))
val n = 10
}