Skip to content

Fix missing changesParents in PostTyper #20062

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 5 commits into from
Apr 11, 2024
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
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about this change? @odersky

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that looks good to me.


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)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this could have unintended consecuences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously we did something wrong --- otherwise, the tree and symbol info will not be in sync.

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)