Skip to content

Commit

Permalink
Introduce fast and full check modes
Browse files Browse the repository at this point in the history
  • Loading branch information
liufengyun committed Nov 29, 2020
1 parent fb88ebf commit 299e048
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 48 deletions.
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class ScalaSettings extends Settings.SettingGroup {
val YnoKindPolymorphism: Setting[Boolean] = BooleanSetting("-Yno-kind-polymorphism", "Disable kind polymorphism.")
val YexplicitNulls: Setting[Boolean] = BooleanSetting("-Yexplicit-nulls", "Make reference types non-nullable. Nullable types can be expressed with unions: e.g. String|Null.")
val YerasedTerms: Setting[Boolean] = BooleanSetting("-Yerased-terms", "Allows the use of erased terms.")
val YcheckInit: Setting[Boolean] = BooleanSetting("-Ycheck-init", "Check initialization of objects")
val YcheckInit: Setting[String] = ChoiceSetting("-Ycheck-init", "mode", "Check initialization of objects: fast | full", List("fast", "full"), "")
val YrequireTargetName: Setting[Boolean] = BooleanSetting("-Yrequire-targetName", "Warn if an operator is defined without a @targetName annotation")

/** Area-specific debug output */
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ object Symbols {
ctx.settings.YretainTrees.value ||
denot.owner.isTerm || // no risk of leaking memory after a run for these
denot.isOneOf(InlineOrProxy) || // need to keep inline info
ctx.settings.YcheckInit.value // initialization check
ctx.settings.YcheckInit.value.nonEmpty // initialization check

/** The last denotation of this symbol */
private var lastDenot: SymDenotation = _
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Checker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Checker extends MiniPhase {
override val runsAfter = Set(Pickler.name)

override def isEnabled(using Context): Boolean =
super.isEnabled && ctx.settings.YcheckInit.value
super.isEnabled && ctx.settings.YcheckInit.value.nonEmpty

override def transformTypeDef(tree: TypeDef)(using Context): tpd.Tree = {
if (!tree.isClassDef) return tree
Expand All @@ -47,7 +47,6 @@ class Checker extends MiniPhase {
// A concrete class may not be instantiated if the self type is not satisfied
if (instantiable) {
implicit val state: Checking.State = Checking.State(
checking = Set.empty,
checked = Set.empty,
path = Vector.empty,
thisClass = cls,
Expand Down
41 changes: 20 additions & 21 deletions compiler/src/dotty/tools/dotc/transform/init/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ object Checking {
*/

case class State(
var checking: Set[Effect], // effects that are being checked
var checked: Set[Effect], // effects that have been checked
var checked: Set[Effect], // effects that have been checked or are being 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 @@ -51,10 +50,8 @@ object Checking {

def test(op: State ?=> Errors): Errors = {
val savedChecked = checked
val savedChecking = checking
val errors = op(using this)
checked = savedChecked
checking = savedChecking
errors
}
}
Expand All @@ -66,24 +63,19 @@ object Checking {
private val defaultCheck: (Effect, State) => Errors = { (eff: Effect, state: State) =>
given State = state
trace("checking effect " + eff.show, init, errs => Errors.show(errs.asInstanceOf[Errors])) {
if (state.checked.contains(eff) || state.checking.contains(eff)) {
if (state.checked.contains(eff)) {
traceIndented("Already checked " + eff.show, init)
Errors.empty
}
else {
state.checking = state.checking + eff
state.checked = state.checked + eff
val state2: State = state.copy(path = state.path :+ eff.source)
val errors =
eff match {
case eff: Promote => Checking.checkPromote(eff)(using state2)
case eff: FieldAccess => Checking.checkFieldAccess(eff)(using state2)
case eff: MethodCall => Checking.checkMethodCall(eff)(using state2)
case eff: AccessGlobal => Checking.checkAccessGlobal(eff)(using state2)
}

state.checking -= eff
state.checked += eff
errors
eff match {
case eff: Promote => Checking.checkPromote(eff)(using state2)
case eff: FieldAccess => Checking.checkFieldAccess(eff)(using state2)
case eff: MethodCall => Checking.checkMethodCall(eff)(using state2)
case eff: AccessGlobal => Checking.checkAccessGlobal(eff)(using state2)
}
}
}
}
Expand Down Expand Up @@ -204,14 +196,21 @@ object Checking {
case warm: Warm => warm.classSymbol.typeRef
case _: ThisRef => ??? // impossible

// Fast check of public methods directly defined in class
// Fast check of public term members directly defined in class
// If errors found, there is no need to go further
classRef.decls.foreach { m =>
if m.is(Flags.Method, butNot = Flags.Private) && m.hasSource then
val errors = state.check(MethodCall(pot, m)(source))
classRef.decls.foreach { sym =>
if sym.is(Flags.Method, butNot = Flags.Private) && !sym.isConstructor && sym.hasSource then
val errors = state.check(MethodCall(pot, sym)(source))
if errors.nonEmpty then return errors

// if sym.name.isTermName && !sym.isOneOf(Flags.Method | Flags.Private) && sym.hasSource then
// val errors = state.check(Promote(FieldReturn(pot, sym)(source))(source))
// if errors.nonEmpty then return errors
}

// best-effort in fast mode
// if !theEnv.isFullMode then return Errors.empty

val excludedFlags = Flags.Deferred | Flags.Private | Flags.Protected

classRef.fields.foreach { denot =>
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/init/Env.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ case class Env(ctx: Context) {

def withOwner(owner: Symbol) = this.copy(ctx = this.ctx.withOwner(owner))

def checkGlobal: Boolean = false
def isFullMode: Boolean = ctx.settings.YcheckInit.value == "full"

/** Whether values of a given type is always fully initialized?
*
Expand Down
10 changes: 5 additions & 5 deletions compiler/src/dotty/tools/dotc/transform/init/Summarization.scala
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ object Summarization {
val Summary(pots, effs) = resolveThis(enclosing, thisRef, cur, expr)
val summary = Summary(effs)
if pots.isEmpty then
if env.checkGlobal then summary + LocalHot(cls)(expr)
if env.isFullMode then summary + LocalHot(cls)(expr)
else summary
else {
assert(pots.size == 1)
Expand All @@ -102,13 +102,13 @@ object Summarization {
else {
val summary = analyze(tref.prefix, expr)
if summary.pots.isEmpty then
if env.checkGlobal then summary + LocalHot(cls)(expr)
if env.isFullMode then summary + LocalHot(cls)(expr)
else summary
else {
assert(summary.pots.size == 1)
val pot = summary.pots.head
val res = summary.dropPotentials
if env.checkGlobal && Potentials.isGlobalPath(pot) then
if env.isFullMode && Potentials.isGlobalPath(pot) then
res + LocalHot(cls)(expr)
else
res + Warm(cls, pot)(expr)
Expand Down Expand Up @@ -233,7 +233,7 @@ object Summarization {
Summary.empty

case tmref: TermRef
if env.checkGlobal && tmref.symbol.is(Flags.Module, butNot = Flags.Package) && tmref.symbol.isStatic =>
if env.isFullMode && tmref.symbol.is(Flags.Module, butNot = Flags.Package) && tmref.symbol.isStatic =>
val cls = tmref.symbol.moduleClass
if cls.primaryConstructor.exists then
val pot = Global(tmref)(source)
Expand Down Expand Up @@ -291,7 +291,7 @@ object Summarization {
val enclosing = cur.owner.lexicallyEnclosingClass.asClass
// Dotty uses O$.this outside of the object O
if (enclosing.is(Flags.Package) && cls.is(Flags.Module))
if env.checkGlobal && cls.primaryConstructor.exists then // ctor may not exist, tests/init/neg/inner19
if env.isFullMode && cls.primaryConstructor.exists then // ctor may not exist, tests/init/neg/inner19
val pot = Global(cls.sourceModule.termRef)(source)
return Summary(pot) + AccessGlobal(pot) + MethodCall(pot, cls.primaryConstructor)(source)
else
Expand Down
31 changes: 15 additions & 16 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -306,24 +306,23 @@ class CompilationTests {
}.checkRuns()

// initialization tests
@Test def checkInitNeg: Unit = {
implicit val testGroup: TestGroup = TestGroup("checkInit")
val options = defaultOptions.and("-Ycheck-init", "-Xfatal-warnings")
compileFilesInDir("tests/init/neg/", options)
}.checkExpectedErrors()

@Test def checkInitCrash: Unit = {
implicit val testGroup: TestGroup = TestGroup("checkInit")
val options = defaultOptions.and("-Ycheck-init")
compileFilesInDir("tests/init/crash", options)
}.checkCompile()
@Test def checkInitFast: Unit = {
implicit val testGroup: TestGroup = TestGroup("checkInitFast")
val options = defaultOptions.and("-Ycheck-init", "fast", "-Xfatal-warnings")
compileFilesInDir("tests/init/neg", options).checkExpectedErrors()
compileFilesInDir("tests/init/full/neg", options).checkCompile()
compileFilesInDir("tests/init/pos", options).checkCompile()
compileFilesInDir("tests/init/crash", options.without("-Xfatal-warnings")).checkCompile()
}

@Test def checkInitPos: Unit = {
@Test def checkInitFull: Unit = {
implicit val testGroup: TestGroup = TestGroup("checkInit")
val options = defaultOptions.and("-Ycheck-init", "-Xfatal-warnings")
compileFilesInDir("tests/init/pos", options)
}.checkCompile()

val options = defaultOptions.and("-Ycheck-init", "full", "-Xfatal-warnings")
compileFilesInDir("tests/init/neg", options).checkExpectedErrors()
compileFilesInDir("tests/init/full/neg", options).checkExpectedErrors()
compileFilesInDir("tests/init/pos", options).checkCompile()
compileFilesInDir("tests/init/crash", options.without("-Xfatal-warnings")).checkCompile()
}
}

object CompilationTests extends ParallelTesting {
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) // leak is OK, accessing `f` is reported below
}

class C extends A {
Expand Down

0 comments on commit 299e048

Please sign in to comment.