Skip to content

Commit

Permalink
Fix missing changesParents in PostTyper (scala#20062)
Browse files Browse the repository at this point in the history
The following two problems lead to crash of -Ysafe-init-global while
compiling scala2-library-bootstrapped:

- Fix missing changesParents in PostTyper
- Fix mismatch of parents tree and parents in symbol info
  • Loading branch information
liufengyun authored Apr 11, 2024
2 parents b096764 + a3d0098 commit 62e0641
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 14 deletions.
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,13 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
if (ctx.settings.YtestPickler.value) List("pickler")
else ctx.settings.YstopAfter.value

val runCtx = ctx.fresh
runCtx.setProfiler(Profiler())

val pluginPlan = ctx.base.addPluginPhases(ctx.base.phasePlan)
val phases = ctx.base.fusePhases(pluginPlan,
ctx.settings.Yskip.value, ctx.settings.YstopBefore.value, stopAfter, ctx.settings.Ycheck.value)
ctx.base.usePhases(phases)
ctx.base.usePhases(phases, runCtx)

if ctx.settings.YnoDoubleBindings.value then
ctx.base.checkNoDoubleBindings = true
Expand Down Expand Up @@ -340,9 +343,6 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
profiler.finished()
}

val runCtx = ctx.fresh
runCtx.setProfiler(Profiler())
unfusedPhases.foreach(_.initContext(runCtx))
val fusedPhases = runCtx.base.allPhases
if ctx.settings.explainCyclic.value then
runCtx.setProperty(CyclicReference.Trace, new CyclicReference.Trace())
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ object Contexts {
val definitions: Definitions = new Definitions

// Set up some phases to get started */
usePhases(List(SomePhase))
usePhases(List(SomePhase), FreshContext(this))

/** Initializes the `ContextBase` with a starting context.
* This initializes the `platform` and the `definitions`.
Expand Down
16 changes: 13 additions & 3 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ object Phases {
* The list should never contain NoPhase.
* if fusion is enabled, phases in same subgroup will be fused to single phase.
*/
final def usePhases(phasess: List[Phase], fuse: Boolean = true): Unit = {
final def usePhases(phasess: List[Phase], runCtx: FreshContext, fuse: Boolean = true): Unit = {

val flatPhases = collection.mutable.ListBuffer[Phase]()

Expand Down Expand Up @@ -161,11 +161,21 @@ object Phases {
phase match {
case p: MegaPhase =>
val miniPhases = p.miniPhases
miniPhases.foreach{ phase =>
for phase <- miniPhases do
checkRequirements(phase)
phase.init(this, nextPhaseId)}
// Given phases a chance to initialize state based on the run context.
//
// `phase.initContext` should be called before `phase.init` as the later calls abstract methods
// `changesMembers` and `changeParents` which may depend on the run context.
//
// See `PostTyper.changeParents`
phase.initContext(runCtx)
phase.init(this, nextPhaseId)
end for
p.init(this, miniPhases.head.id, miniPhases.last.id)
case _ =>
// See comment above about the ordering of the two calls.
phase.initContext(runCtx)
phase.init(this, nextPhaseId)
checkRequirements(phase)
}
Expand Down
20 changes: 19 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,29 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>

override def changesMembers: Boolean = true // the phase adds super accessors and synthetic members

/**
* Serializable and AbstractFunction1 are added for companion objects of case classes in scala2-library
*/
override def changesParents: Boolean =
if !initContextCalled then
throw new Exception("Calling changesParents before initContext, should call initContext first")
compilingScala2StdLib

override def transformPhase(using Context): Phase = thisPhase.next

def newTransformer(using Context): Transformer =
new PostTyperTransformer

/**
* Used to check that `changesParents` is called after `initContext`.
*
* This contract is easy to break and results in subtle bugs.
*/
private var initContextCalled = false

private var compilingScala2StdLib = false
override def initContext(ctx: FreshContext): Unit =
initContextCalled = true
compilingScala2StdLib = ctx.settings.YcompileScala2Library.value(using ctx)

val superAcc: SuperAccessors = new SuperAccessors(thisPhase)
Expand Down Expand Up @@ -576,7 +592,9 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
if !sym.hasAnnotation(defn.ExperimentalAnnot) && ctx.settings.experimental.value && isTopLevelDefinitionInSource(sym) then
sym.addAnnotation(ExperimentalAnnotation("Added by -experimental", sym.span))

private def scala2LibPatch(tree: TypeDef)(using Context) =
// It needs to run at the phase of the postTyper --- otherwise, the test of the symbols will use
// the transformed denotation with added `Serializable` and `AbstractFunction1`.
private def scala2LibPatch(tree: TypeDef)(using Context) = atPhase(thisPhase):
val sym = tree.symbol
if compilingScala2StdLib && sym.is(ModuleClass) then
// Add Serializable to companion objects of serializable classes,
Expand Down
10 changes: 7 additions & 3 deletions compiler/src/dotty/tools/dotc/transform/init/Checker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,16 @@ class Checker extends Phase:
traverser.traverse(unit.tpdTree)

override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] =
val checkCtx = ctx.fresh.setPhase(this.start)
val checkCtx = ctx.fresh.setPhase(this)
val traverser = new InitTreeTraverser()
val unitContexts = units.map(unit => checkCtx.fresh.setCompilationUnit(unit))

val units0 =
for unitContext <- unitContexts if traverse(traverser)(using unitContext) yield unitContext.compilationUnit
for
unit <- units
unitContext = checkCtx.fresh.setCompilationUnit(unit)
if traverse(traverser)(using unitContext)
yield
unitContext.compilationUnit

cancellable {
val classes = traverser.getClasses()
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Objects.scala
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import StdNames.*
import Names.TermName
import NameKinds.OuterSelectName
import NameKinds.SuperAccessorName
import Decorators.*

import ast.tpd.*
import util.{ SourcePosition, NoSourcePosition }
Expand Down Expand Up @@ -66,12 +67,11 @@ import dotty.tools.dotc.core.Flags.AbstractOrTrait
* whole-program analysis. However, the check is not modular in terms of project boundaries.
*
*/
import Decorators.*
class Objects(using Context @constructorOnly):
val immutableHashSetBuider: Symbol = requiredClass("scala.collection.immutable.HashSetBuilder")
// TODO: this should really be an annotation on the rhs of the field initializer rather than the field itself.
val HashSetBuilder_rootNode: Symbol = immutableHashSetBuider.requiredValue("rootNode")

val whiteList = Set(HashSetBuilder_rootNode)

// ----------------------------- abstract domain -----------------------------
Expand Down
1 change: 1 addition & 0 deletions project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,7 @@ object Build {
settings(commonBootstrappedSettings).
settings(scala2LibraryBootstrappedSettings).
settings(moduleName := "scala2-library")
// -Ycheck:all is set in project/scripts/scala2-library-tasty-mima.sh

/** Scala 2 library compiled by dotty using the latest published sources of the library.
*
Expand Down
2 changes: 2 additions & 0 deletions tests/init-global/pos/scala2-library.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
//> using options -Ycompile-scala2-library
case class UninitializedFieldError(msg: String) extends RuntimeException(msg)

0 comments on commit 62e0641

Please sign in to comment.