Skip to content

Commit 62e0641

Browse files
authored
Fix missing changesParents in PostTyper (#20062)
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
2 parents b096764 + a3d0098 commit 62e0641

File tree

8 files changed

+49
-14
lines changed

8 files changed

+49
-14
lines changed

Diff for: compiler/src/dotty/tools/dotc/Run.scala

+4-4
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,13 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
293293
if (ctx.settings.YtestPickler.value) List("pickler")
294294
else ctx.settings.YstopAfter.value
295295

296+
val runCtx = ctx.fresh
297+
runCtx.setProfiler(Profiler())
298+
296299
val pluginPlan = ctx.base.addPluginPhases(ctx.base.phasePlan)
297300
val phases = ctx.base.fusePhases(pluginPlan,
298301
ctx.settings.Yskip.value, ctx.settings.YstopBefore.value, stopAfter, ctx.settings.Ycheck.value)
299-
ctx.base.usePhases(phases)
302+
ctx.base.usePhases(phases, runCtx)
300303

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

343-
val runCtx = ctx.fresh
344-
runCtx.setProfiler(Profiler())
345-
unfusedPhases.foreach(_.initContext(runCtx))
346346
val fusedPhases = runCtx.base.allPhases
347347
if ctx.settings.explainCyclic.value then
348348
runCtx.setProperty(CyclicReference.Trace, new CyclicReference.Trace())

Diff for: compiler/src/dotty/tools/dotc/core/Contexts.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ object Contexts {
892892
val definitions: Definitions = new Definitions
893893

894894
// Set up some phases to get started */
895-
usePhases(List(SomePhase))
895+
usePhases(List(SomePhase), FreshContext(this))
896896

897897
/** Initializes the `ContextBase` with a starting context.
898898
* This initializes the `platform` and the `definitions`.

Diff for: compiler/src/dotty/tools/dotc/core/Phases.scala

+13-3
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ object Phases {
126126
* The list should never contain NoPhase.
127127
* if fusion is enabled, phases in same subgroup will be fused to single phase.
128128
*/
129-
final def usePhases(phasess: List[Phase], fuse: Boolean = true): Unit = {
129+
final def usePhases(phasess: List[Phase], runCtx: FreshContext, fuse: Boolean = true): Unit = {
130130

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

@@ -161,11 +161,21 @@ object Phases {
161161
phase match {
162162
case p: MegaPhase =>
163163
val miniPhases = p.miniPhases
164-
miniPhases.foreach{ phase =>
164+
for phase <- miniPhases do
165165
checkRequirements(phase)
166-
phase.init(this, nextPhaseId)}
166+
// Given phases a chance to initialize state based on the run context.
167+
//
168+
// `phase.initContext` should be called before `phase.init` as the later calls abstract methods
169+
// `changesMembers` and `changeParents` which may depend on the run context.
170+
//
171+
// See `PostTyper.changeParents`
172+
phase.initContext(runCtx)
173+
phase.init(this, nextPhaseId)
174+
end for
167175
p.init(this, miniPhases.head.id, miniPhases.last.id)
168176
case _ =>
177+
// See comment above about the ordering of the two calls.
178+
phase.initContext(runCtx)
169179
phase.init(this, nextPhaseId)
170180
checkRequirements(phase)
171181
}

Diff for: compiler/src/dotty/tools/dotc/transform/PostTyper.scala

+19-1
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,29 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
7676

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

79+
/**
80+
* Serializable and AbstractFunction1 are added for companion objects of case classes in scala2-library
81+
*/
82+
override def changesParents: Boolean =
83+
if !initContextCalled then
84+
throw new Exception("Calling changesParents before initContext, should call initContext first")
85+
compilingScala2StdLib
86+
7987
override def transformPhase(using Context): Phase = thisPhase.next
8088

8189
def newTransformer(using Context): Transformer =
8290
new PostTyperTransformer
8391

92+
/**
93+
* Used to check that `changesParents` is called after `initContext`.
94+
*
95+
* This contract is easy to break and results in subtle bugs.
96+
*/
97+
private var initContextCalled = false
98+
8499
private var compilingScala2StdLib = false
85100
override def initContext(ctx: FreshContext): Unit =
101+
initContextCalled = true
86102
compilingScala2StdLib = ctx.settings.YcompileScala2Library.value(using ctx)
87103

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

579-
private def scala2LibPatch(tree: TypeDef)(using Context) =
595+
// It needs to run at the phase of the postTyper --- otherwise, the test of the symbols will use
596+
// the transformed denotation with added `Serializable` and `AbstractFunction1`.
597+
private def scala2LibPatch(tree: TypeDef)(using Context) = atPhase(thisPhase):
580598
val sym = tree.symbol
581599
if compilingScala2StdLib && sym.is(ModuleClass) then
582600
// Add Serializable to companion objects of serializable classes,

Diff for: compiler/src/dotty/tools/dotc/transform/init/Checker.scala

+7-3
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,16 @@ class Checker extends Phase:
3636
traverser.traverse(unit.tpdTree)
3737

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

4342
val units0 =
44-
for unitContext <- unitContexts if traverse(traverser)(using unitContext) yield unitContext.compilationUnit
43+
for
44+
unit <- units
45+
unitContext = checkCtx.fresh.setCompilationUnit(unit)
46+
if traverse(traverser)(using unitContext)
47+
yield
48+
unitContext.compilationUnit
4549

4650
cancellable {
4751
val classes = traverser.getClasses()

Diff for: compiler/src/dotty/tools/dotc/transform/init/Objects.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import StdNames.*
1111
import Names.TermName
1212
import NameKinds.OuterSelectName
1313
import NameKinds.SuperAccessorName
14+
import Decorators.*
1415

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

7777
// ----------------------------- abstract domain -----------------------------

Diff for: project/Build.scala

+1
Original file line numberDiff line numberDiff line change
@@ -1053,6 +1053,7 @@ object Build {
10531053
settings(commonBootstrappedSettings).
10541054
settings(scala2LibraryBootstrappedSettings).
10551055
settings(moduleName := "scala2-library")
1056+
// -Ycheck:all is set in project/scripts/scala2-library-tasty-mima.sh
10561057

10571058
/** Scala 2 library compiled by dotty using the latest published sources of the library.
10581059
*

Diff for: tests/init-global/pos/scala2-library.scala

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
//> using options -Ycompile-scala2-library
2+
case class UninitializedFieldError(msg: String) extends RuntimeException(msg)

0 commit comments

Comments
 (0)