Skip to content

Initialization checker improvement #9335

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 22 commits into from
Aug 26, 2020
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
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Checker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class Checker extends MiniPhase {
val phaseName = "initChecker"

// cache of class summary
private val baseEnv = Env(null, mutable.Map.empty)
private val baseEnv = Env(null)

override val runsAfter = Set(Pickler.name)

Expand Down Expand Up @@ -52,7 +52,7 @@ class Checker extends MiniPhase {
thisClass = cls,
fieldsInited = mutable.Set.empty,
parentsInited = mutable.Set.empty,
env = baseEnv.withCtx(ctx)
env = baseEnv.withCtx(ctx.withOwner(cls))
)

Checking.checkClassBody(tree)
Expand Down
131 changes: 56 additions & 75 deletions compiler/src/dotty/tools/dotc/transform/init/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ object Checking {
*
*/
case class State(
visited: mutable.Set[Effect], // effects that have been expanded
visited: mutable.Set[Effect], // effects that have been checked
path: Vector[Tree], // the path that leads to the current effect
thisClass: ClassSymbol, // the concrete class of `this`
fieldsInited: mutable.Set[Symbol],
Expand All @@ -41,6 +41,8 @@ object Checking {
visited += eff
copy(path = this.path :+ eff.source)
}

def withOwner(sym: Symbol): State = copy(env = env.withOwner(sym))
}

private implicit def theEnv(implicit state: State): Env = state.env
Expand All @@ -51,17 +53,19 @@ object Checking {
* However, summarization can be done lazily on-demand to improve
* performance.
*/
def checkClassBody(cdef: TypeDef)(implicit state: State): Unit = traceOp("checking " + cdef.symbol.show, init) {
def checkClassBody(cdef: TypeDef)(implicit state: State): Unit = {
traceIndented("\n\n>>>> checking " + cdef.symbol.show, init)

val cls = cdef.symbol.asClass
val tpl = cdef.rhs.asInstanceOf[Template]

// mark current class as initialized, required for linearization
state.parentsInited += cls

def checkClassBodyStat(tree: Tree)(using Context): Unit = traceOp("checking " + tree.show, init) {
def checkClassBodyStat(tree: Tree)(implicit state: State): Unit = traceOp("checking " + tree.show, init) {
tree match {
case vdef : ValDef =>
val (pots, effs) = Summarization.analyze(vdef.rhs)(theEnv.withOwner(vdef.symbol))
val (pots, effs) = Summarization.analyze(vdef.rhs)
theEnv.summaryOf(cls).cacheFor(vdef.symbol, (pots, effs))
if (!vdef.symbol.is(Flags.Lazy)) {
checkEffectsIn(effs, cls)
Expand All @@ -79,15 +83,31 @@ object Checking {
// see spec 5.1 about "Template Evaluation".
// https://www.scala-lang.org/files/archive/spec/2.13/05-classes-and-objects.html

def checkCtor(ctor: Symbol, tp: Type, source: Tree)(using Context): Unit = {
def checkConstructor(ctor: Symbol, tp: Type, source: Tree)(implicit state: State): Unit = traceOp("checking " + ctor.show, init) {
val cls = ctor.owner
val classDef = cls.defTree
if (!classDef.isEmpty) {
if (ctor.isPrimaryConstructor) checkClassBody(classDef.asInstanceOf[TypeDef])
else checkSecondaryConstructor(ctor)
if (ctor.isPrimaryConstructor) checkClassBody(classDef.asInstanceOf[TypeDef])(state.withOwner(cls))
else checkSecondaryConstructor(ctor)(state.withOwner(cls))
}
}

def checkSecondaryConstructor(ctor: Symbol)(implicit state: State): Unit = traceOp("checking " + ctor.show, init) {
val Block(ctorCall :: stats, expr) = ctor.defTree.asInstanceOf[DefDef].rhs
val cls = ctor.owner.asClass

traceOp("check ctor: " + ctorCall.show, init) {
val ctor = ctorCall.symbol
if (ctor.isPrimaryConstructor)
checkClassBody(cls.defTree.asInstanceOf[TypeDef])
else
checkSecondaryConstructor(ctor)
}

(stats :+ expr).foreach { stat =>
val (_, effs) = Summarization.analyze(stat)(theEnv.withOwner(ctor))
checkEffectsIn(effs, cls)
}
else if (!cls.isOneOf(Flags.EffectivelyOpenFlags))
report.warning("Inheriting non-open class may cause initialization errors", source.srcPos)
}

cls.paramAccessors.foreach { acc =>
Expand All @@ -100,61 +120,42 @@ object Checking {
tpl.parents.foreach {
case tree @ Block(_, parent) =>
val (ctor, _, _) = decomposeCall(parent)
checkCtor(ctor.symbol, parent.tpe, tree)
checkConstructor(ctor.symbol, parent.tpe, tree)

case tree @ Apply(Block(_, parent), _) =>
val (ctor, _, _) = decomposeCall(parent)
checkCtor(ctor.symbol, tree.tpe, tree)
checkConstructor(ctor.symbol, tree.tpe, tree)

case parent : Apply =>
val (ctor, _, argss) = decomposeCall(parent)
checkCtor(ctor.symbol, parent.tpe, parent)
checkConstructor(ctor.symbol, parent.tpe, parent)

case ref =>
val cls = ref.tpe.classSymbol.asClass
if (!state.parentsInited.contains(cls) && cls.primaryConstructor.exists)
checkCtor(cls.primaryConstructor, ref.tpe, ref)
checkConstructor(cls.primaryConstructor, ref.tpe, ref)
}

// check class body
tpl.body.foreach { checkClassBodyStat(_) }
}

def checkSecondaryConstructor(ctor: Symbol)(implicit state: State): Unit = traceOp("checking " + ctor.show, init) {
val Block(ctorCall :: stats, expr) = ctor.defTree.asInstanceOf[DefDef].rhs
val cls = ctor.owner.asClass

traceOp("check ctor: " + ctorCall.show, init) {
val ctor = ctorCall.symbol
if (ctor.isPrimaryConstructor)
checkClassBody(cls.defTree.asInstanceOf[TypeDef])
else
checkSecondaryConstructor(ctor)
}

(stats :+ expr).foreach { stat =>
val (_, effs) = Summarization.analyze(stat)(theEnv.withOwner(ctor))
checkEffectsIn(effs, cls)
}
}

private def checkEffectsIn(effs: Effects, cls: ClassSymbol)(implicit state: State): Unit = traceOp("checking effects " + Effects.show(effs), init) {
val rebased = Effects.asSeenFrom(effs, ThisRef(state.thisClass)(null), cls, Potentials.empty)
for {
eff <- rebased
eff <- effs
error <- check(eff)
} error.issue
}

private def check(eff: Effect)(implicit state: State): Errors =
if (state.visited.contains(eff)) Errors.empty else trace("checking effect " + eff.show, init, errs => Errors.show(errs.asInstanceOf[Errors])) {
if (state.visited.contains(eff)) Errors.empty
else trace("checking effect " + eff.show, init, errs => Errors.show(errs.asInstanceOf[Errors])) {
implicit val state2: State = state.withVisited(eff)

eff match {
case Promote(pot) =>
pot match {
case pot @ ThisRef(cls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)
case pot: ThisRef =>
PromoteThis(pot, eff.source, state2.path).toErrors

case _: Cold =>
Expand All @@ -180,18 +181,14 @@ object Checking {
case FieldAccess(pot, field) =>

pot match {
case ThisRef(cls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolve(cls, field)
case _: ThisRef =>
val target = resolve(state.thisClass, field)
if (target.is(Flags.Lazy)) check(MethodCall(pot, target)(eff.source))
else if (!state.fieldsInited.contains(target)) AccessNonInit(target, state2.path).toErrors
else Errors.empty

case SuperRef(ThisRef(cls), supercls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolveSuper(cls, supercls, field)
case SuperRef(_: ThisRef, supercls) =>
val target = resolveSuper(state.thisClass, supercls, field)
if (target.is(Flags.Lazy)) check(MethodCall(pot, target)(eff.source))
else if (!state.fieldsInited.contains(target)) AccessNonInit(target, state2.path).toErrors
else Errors.empty
Expand All @@ -217,10 +214,8 @@ object Checking {

case MethodCall(pot, sym) =>
pot match {
case thisRef @ ThisRef(cls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolve(cls, sym)
case thisRef: ThisRef =>
val target = resolve(state.thisClass, sym)
if (!target.isOneOf(Flags.Method | Flags.Lazy))
check(FieldAccess(pot, target)(eff.source))
else if (target.isInternal) {
Expand All @@ -229,10 +224,8 @@ object Checking {
}
else CallUnknown(target, eff.source, state2.path).toErrors

case SuperRef(thisRef @ ThisRef(cls), supercls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolveSuper(cls, supercls, sym)
case SuperRef(thisRef: ThisRef, supercls) =>
val target = resolveSuper(state.thisClass, supercls, sym)
if (!target.is(Flags.Method))
check(FieldAccess(pot, target)(eff.source))
else if (target.isInternal) {
Expand Down Expand Up @@ -272,17 +265,13 @@ object Checking {
pot match {
case MethodReturn(pot1, sym) =>
pot1 match {
case thisRef @ ThisRef(cls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolve(cls, sym)
case thisRef: ThisRef =>
val target = resolve(state.thisClass, sym)
if (target.isInternal) (thisRef.potentialsOf(target), Effects.empty)
else Summary.empty // warning already issued in call effect

case SuperRef(thisRef @ ThisRef(cls), supercls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolveSuper(cls, supercls, sym)
case SuperRef(thisRef: ThisRef, supercls) =>
val target = resolveSuper(state.thisClass, supercls, sym)
if (target.isInternal) (thisRef.potentialsOf(target), Effects.empty)
else Summary.empty // warning already issued in call effect

Expand Down Expand Up @@ -314,17 +303,13 @@ object Checking {

case FieldReturn(pot1, sym) =>
pot1 match {
case thisRef @ ThisRef(cls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolve(cls, sym)
case thisRef: ThisRef =>
val target = resolve(state.thisClass, sym)
if (sym.isInternal) (thisRef.potentialsOf(target), Effects.empty)
else (Cold()(pot.source).toPots, Effects.empty)

case SuperRef(thisRef @ ThisRef(cls), supercls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

val target = resolveSuper(cls, supercls, sym)
case SuperRef(thisRef: ThisRef, supercls) =>
val target = resolveSuper(state.thisClass, supercls, sym)
if (target.isInternal) (thisRef.potentialsOf(target), Effects.empty)
else (Cold()(pot.source).toPots, Effects.empty)

Expand All @@ -347,19 +332,15 @@ object Checking {

case Outer(pot1, cls) =>
pot1 match {
case ThisRef(cls) =>
assert(cls == state.thisClass, "unexpected potential " + pot.show)

case _: ThisRef =>
// all outers for `this` are assumed to be hot
Summary.empty

case _: Fun =>
throw new Exception("Unexpected code reached")

case warm: Warm =>
(warm.outerFor(cls), Effects.empty)

case _: Cold =>
throw new Exception("Unexpected code reached")
(warm.resolveOuter(cls), Effects.empty)

case _ =>
val (pots, effs) = expand(pot1)
Expand Down
36 changes: 16 additions & 20 deletions compiler/src/dotty/tools/dotc/transform/init/Effects.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ object Effects {
def show(effs: Effects)(using Context): String =
effs.map(_.show).mkString(", ")

/** Effects that are related to safe initialization */
/** Effects that are related to safe initialization performed on potentials */
sealed trait Effect {
def size: Int
def potential: Potential

def show(using Context): String

def source: Tree
}

Expand All @@ -37,48 +39,42 @@ object Effects {
* - the selection chain on a potential is too long
*/
case class Promote(potential: Potential)(val source: Tree) extends Effect {
def size: Int = potential.size
def show(using Context): String =
potential.show + "↑"
def show(using Context): String = potential.show + "↑"
}

/** Field access, `a.f` */
case class FieldAccess(potential: Potential, field: Symbol)(val source: Tree) extends Effect {
assert(field != NoSymbol)

def size: Int = potential.size
def show(using Context): String =
potential.show + "." + field.name.show + "!"
def show(using Context): String = potential.show + "." + field.name.show + "!"
}

/** Method call, `a.m()` */
case class MethodCall(potential: Potential, method: Symbol)(val source: Tree) extends Effect {
assert(method != NoSymbol)

def size: Int = potential.size
def show(using Context): String = potential.show + "." + method.name.show + "!"
}

// ------------------ operations on effects ------------------

extension (eff: Effect) def toEffs: Effects = Effects.empty + eff

def asSeenFrom(eff: Effect, thisValue: Potential, currentClass: ClassSymbol, outer: Potentials)(implicit env: Env): Effects =
trace(eff.show + " asSeenFrom " + thisValue.show + ", current = " + currentClass.show + ", outer = " + Potentials.show(outer), init, effs => show(effs.asInstanceOf[Effects])) { eff match {
def asSeenFrom(eff: Effect, thisValue: Potential)(implicit env: Env): Effect =
trace(eff.show + " asSeenFrom " + thisValue.show + ", current = " + currentClass.show, init, effs => show(effs.asInstanceOf[Effects])) { eff match {
case Promote(pot) =>
Potentials.asSeenFrom(pot, thisValue, currentClass, outer).promote(eff.source)
val pot1 = Potentials.asSeenFrom(pot, thisValue)
Promote(pot1)(eff.source)

case FieldAccess(pot, field) =>
Potentials.asSeenFrom(pot, thisValue, currentClass, outer).map { pot =>
FieldAccess(pot, field)(eff.source)
}
val pot1 = Potentials.asSeenFrom(pot, thisValue)
FieldAccess(pot1, field)(eff.source)

case MethodCall(pot, sym) =>
Potentials.asSeenFrom(pot, thisValue, currentClass, outer).map { pot =>
MethodCall(pot, sym)(eff.source)
}
val pot1 = Potentials.asSeenFrom(pot, thisValue)
MethodCall(pot1, sym)(eff.source)
} }

def asSeenFrom(effs: Effects, thisValue: Potential, currentClass: ClassSymbol, outer: Potentials)(implicit env: Env): Effects =
effs.flatMap(asSeenFrom(_, thisValue, currentClass, outer))
def asSeenFrom(effs: Effects, thisValue: Potential)(implicit env: Env): Effects =
effs.map(asSeenFrom(_, thisValue))
}
17 changes: 15 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Env.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import Effects._, Potentials._, Summary._

implicit def theCtx(implicit env: Env): Context = env.ctx

case class Env(ctx: Context, summaryCache: mutable.Map[ClassSymbol, ClassSummary]) {
case class Env(ctx: Context) {
private implicit def self: Env = this

// Methods that should be ignored in the checking
Expand All @@ -46,12 +46,25 @@ case class Env(ctx: Context, summaryCache: mutable.Map[ClassSymbol, ClassSummary
sym.isPrimitiveValueClass || sym == defn.StringClass
}

/** Summary of a method or field */
/** Summary of a class */
private val summaryCache = mutable.Map.empty[ClassSymbol, ClassSummary]
def summaryOf(cls: ClassSymbol): ClassSummary =
if (summaryCache.contains(cls)) summaryCache(cls)
else trace("summary for " + cls.show, init, s => s.asInstanceOf[ClassSummary].show) {
val summary = Summarization.classSummary(cls)
summaryCache(cls) = summary
summary
}

/** Cache for outer this */
private case class OuterKey(warm: Warm, cls: ClassSymbol)
private val outerCache: mutable.Map[OuterKey, Potentials] = mutable.Map.empty
def resolveOuter(warm: Warm, cls: ClassSymbol)(implicit env: Env): Potentials =
val key = OuterKey(warm, cls)
if (outerCache.contains(key)) outerCache(key)
else {
val pots = Potentials.resolveOuter(warm.classSymbol, warm.outer.toPots, cls)
outerCache(key) = pots
pots
}
}
Loading