Harden Period creation and use of Run/PhaseId

Author: SolalPirelli

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -153,11 +153,11 @@ class Compiler {
     List(new GenBCode) ::             // Generate JVM bytecode
     Nil
 
-  // TODO: Initially 0, so that the first nextRunId call would return InitialRunId == 1
-  // Changing the initial runId from 1 to 0 makes the scala2-library-bootstrap fail to compile,
+  // TODO: Initially InitialRunId - 1, so that the first nextRunId call would return InitialRunId
+  // Setting the initial runId to InitialRunId - 1 makes the scala2-library-bootstrap fail to compile,
   // when the underlying issue is fixed, please update dotc.profiler.RealProfiler.chromeTrace logic
-  private var runId: Int = 1
-  def nextRunId: Int = {
+  private var runId: Periods.RunId = Periods.InitialRunId
+  def nextRunId: Periods.RunId = {
     runId += 1; runId
   }
 

compiler/src/dotty/tools/dotc/Run.scala

@@ -579,11 +579,13 @@ extends ImplicitRunInfo, ConstraintRunInfo, cc.CaptureRunInfo {
     start.setRun(this: @unchecked)
   }
 
-  private var myCtx: Context | Null = rootContext(using ictx)
+  private var myCtx: Context | Null = null
 
   /** The context created for this run */
-  given runContext[Dummy_so_its_a_def]: Context = myCtx.nn
-  assert(runContext.runId <= Periods.MaxPossibleRunId)
+  given runContext[Dummy_so_its_a_def]: Context =
+    if myCtx eq null then myCtx = rootContext(using ictx)
+    assert(myCtx.nn.runId <= Periods.MaxPossibleRunId)
+    myCtx.nn
 }
 
 object Run {

compiler/src/dotty/tools/dotc/cc/Capability.scala

@@ -8,7 +8,7 @@ import util.{SimpleIdentitySet, EqHashMap}
 import util.common.alwaysTrue
 import scala.collection.mutable
 import CCState.*
-import Periods.{NoRunId, RunWidth}
+import Periods.{NoRunId, RunId, RunWidth}
 import compiletime.uninitialized
 import StdNames.nme
 import CaptureSet.{Refs, emptyRefs, VarState}
@@ -48,7 +48,7 @@ import collection.immutable
  */
 object Capabilities:
   opaque type Validity = Int
-  def validId(runId: Int, iterId: Int): Validity =
+  def validId(runId: RunId, iterId: Int): Validity =
     runId + (iterId << RunWidth)
   def currentId(using Context): Validity = validId(ctx.runId, ccState.iterationId)
   val invalid: Validity = validId(NoRunId, 0)

compiler/src/dotty/tools/dotc/core/Contexts.scala

@@ -1055,7 +1055,7 @@ object Contexts {
     private[core] var fusedPhases: Array[Phase] = Array.empty[Phase]
 
     /** Next denotation transformer id */
-    private[core] var nextDenotTransformerId: Array[Int] = uninitialized
+    private[core] var nextDenotTransformerId: Array[Periods.PhaseId] = uninitialized
 
     private[core] var denotTransformers: Array[DenotTransformer] = uninitialized
 

compiler/src/dotty/tools/dotc/core/Denotations.scala

@@ -681,7 +681,7 @@ object Denotations {
      *  There may be several `SingleDenotation`s with different validity
      *  representing the same underlying definition at different phases.
      *  These are called a "flock". Flock members are generated by
-     *  @See current. Flock members are connected in a ring
+     *  `current`. Flock members are connected in a ring
      *  with their `nextInRun` fields.
      *
      *  There are the following invariants concerning flock members
@@ -696,10 +696,10 @@ object Denotations {
      *  of this run.
      */
     def initial: SingleDenotation =
-      if (validFor.firstPhaseId <= 1) this
+      if (validFor.firstPhaseId == FirstPhaseId) this
       else {
         var current = nextInRun
-        while (current.validFor.code > this.validFor.code) current = current.nextInRun
+        while (current.validFor > this.validFor) current = current.nextInRun
         current
       }
 
@@ -784,7 +784,7 @@ object Denotations {
      *  are otherwise undefined.
      */
     def skipRemoved(using Context): SingleDenotation =
-      if (validFor.code <= 0) nextDefined else this
+      if (validFor == Nowhere) nextDefined else this
 
     /** Produce a denotation that is valid for the given context.
      *  Usually called when !(validFor contains ctx.period)
@@ -816,10 +816,10 @@ object Denotations {
         var cur = this
         // search for containing period as long as nextInRun increases.
         var next = nextInRun
-        while next.validFor.code > valid.code && !next.validFor.contains(currentPeriod) do
+        while next.validFor > valid && !next.validFor.contains(currentPeriod) do
           cur = next
           next = next.nextInRun
-        if next.validFor.code > valid.code then
+        if next.validFor > valid then
           // in this case, next.validFor contains currentPeriod
           cur = next
           cur
@@ -875,14 +875,14 @@ object Denotations {
         cur
       end goBack
 
-      if valid.code <= 0 then
+      if valid == Nowhere then
         // can happen if we sit on a stale denotation which has been replaced
         // wholesale by an installAfter; in this case, proceed to the next
         // denotation and try again.
         nextDefined
       else if valid.runId != currentPeriod.runId then
         toNewRun
-      else if currentPeriod.code > valid.code then
+      else if currentPeriod > valid then
         goForward
       else
         goBack
@@ -923,7 +923,7 @@ object Denotations {
      */
     protected def transformAfter(phase: DenotTransformer, f: SymDenotation => SymDenotation)(using Context): Unit = {
       var current = symbol.current
-      while (current.validFor.firstPhaseId < phase.id && (current.nextInRun.validFor.code > current.validFor.code))
+      while (current.validFor.firstPhaseId < phase.id && (current.nextInRun.validFor > current.validFor))
         current = current.nextInRun
       var hasNext = true
       while ((current.validFor.firstPhaseId >= phase.id) && hasNext) {
@@ -932,7 +932,7 @@ object Denotations {
           current1.validFor = current.validFor
           current.replaceWith(current1)
         }
-        hasNext = current1.nextInRun.validFor.code > current1.validFor.code
+        hasNext = current1.nextInRun.validFor > current1.validFor
         current = current1.nextInRun
       }
     }

compiler/src/dotty/tools/dotc/core/Periods.scala

@@ -24,7 +24,7 @@ object Periods {
   /** Are all base types in the current period guaranteed to be the same as in period `p`? */
   def currentHasSameBaseTypesAs(p: Period)(using Context): Boolean =
     val period = ctx.period
-    period.code == p.code ||
+    period == p ||
     period.runId == p.runId &&
       unfusedPhases(period.phaseId).sameBaseTypesStartId ==
       unfusedPhases(p.phaseId).sameBaseTypesStartId
@@ -37,9 +37,8 @@ object Periods {
    *     last phase id:        7 bits
    *     #phases before last:  7 bits
    *
-   *     // Dmitry: sign == 0 isn't actually always true, in some cases phaseId == -1 is used for shifts, that easily creates code < 0
    */
-  class Period(val code: Int) extends AnyVal with Showable {
+  class Period private[Periods] (private val code: Int) extends AnyVal with Showable {
 
     /** The run identifier of this period. */
     def runId: RunId = code >>> (PhaseWidth * 2)
@@ -52,7 +51,7 @@ object Periods {
       (code >>> PhaseWidth) & PhaseMask
 
     /** The first phase of this period */
-    def firstPhaseId: Int = lastPhaseId - (code & PhaseMask)
+    def firstPhaseId: PhaseId = lastPhaseId - (code & PhaseMask)
 
     def containsPhaseId(id: PhaseId): Boolean = firstPhaseId <= id && id <= lastPhaseId
 
@@ -76,7 +75,7 @@ object Periods {
       //  iff r1 == r2 & l1 >= l2 && l1 - d1 <= l2 - d2
       //  q.e.d
       val lastDiff = (code - that.code) >>> PhaseWidth
-      lastDiff + (that.code & PhaseMask ) <= (this.code & PhaseMask)
+      lastDiff + (that.code & PhaseMask) <= (this.code & PhaseMask)
     }
 
     /** Does this period overlap with given period? */
@@ -101,25 +100,31 @@ object Periods {
           this.firstPhaseId min that.firstPhaseId,
           this.lastPhaseId max that.lastPhaseId)
 
+    inline def <(that: Period): Boolean =
+      this.code < that.code
+
+    inline def >(that: Period): Boolean =
+      this.code > that.code
+
     def toText(p: Printer): Text =
       inContext(p.printerContext):
         this match
-          case Nowhere                           => "Nowhere"
-          case InitialPeriod                     => "InitialPeriod"
-          case InvalidPeriod                     => "InvalidPeriod"
-          case Period(NoRunId, 0, PhaseMask)     => s"Period(NoRunId.all)"
-          case Period(runId, 0, PhaseMask)       => s"Period($runId.all)"
-          case Period(runId, p1, pn) if p1 == pn => s"Period($runId.$p1(${ctx.base.phases(p1)}))"
-          case Period(runId, p1, pn)             => s"Period($runId.$p1(${ctx.base.phases(p1)})-$pn(${ctx.base.phases(pn)}))"
+          case Nowhere                                           => "Nowhere"
+          case InitialPeriod                                     => "InitialPeriod"
+          case InvalidPeriod                                     => "InvalidPeriod"
+          case Period(NoRunId, FirstPhaseId, MaxPossiblePhaseId) => s"Period(NoRunId.all)"
+          case Period(runId, FirstPhaseId, MaxPossiblePhaseId)   => s"Period($runId.all)"
+          case Period(runId, p1, pn) if p1 == pn                 => s"Period($runId.$p1(${ctx.base.phases(p1)}))"
+          case Period(runId, p1, pn)                             => s"Period($runId.$p1(${ctx.base.phases(p1)})-$pn(${ctx.base.phases(pn)}))"
 
     override def toString: String = this match
-      case Nowhere                           => "Nowhere"
-      case InitialPeriod                     => "InitialPeriod"
-      case InvalidPeriod                     => "InvalidPeriod"
-      case Period(NoRunId, 0, PhaseMask)     => s"Period(NoRunId.all)"
-      case Period(runId, 0, PhaseMask)       => s"Period($runId.all)"
-      case Period(runId, p1, pn) if p1 == pn => s"Period($runId.$p1)"
-      case Period(runId, p1, pn)             => s"Period($runId.$p1-$pn)"
+      case Nowhere                                           => "Nowhere"
+      case InitialPeriod                                     => "InitialPeriod"
+      case InvalidPeriod                                     => "InvalidPeriod"
+      case Period(NoRunId, FirstPhaseId, MaxPossiblePhaseId) => s"Period(NoRunId.all)"
+      case Period(runId, FirstPhaseId, MaxPossiblePhaseId)   => s"Period($runId.all)"
+      case Period(runId, p1, pn) if p1 == pn                 => s"Period($runId.$p1)"
+      case Period(runId, p1, pn)                             => s"Period($runId.$p1-$pn)"
 
     def ==(that: Period): Boolean = this.code == that.code
     def !=(that: Period): Boolean = this.code != that.code
@@ -137,7 +142,7 @@ object Periods {
 
     /** The interval consisting of all periods of given run id */
     def allInRun(rid: RunId): Period =
-      apply(rid, 0, PhaseMask)
+      apply(rid, FirstPhaseId, MaxPossiblePhaseId)
 
     def unapply(p: Period): Extractor = new Extractor(p.code)
 
@@ -151,13 +156,6 @@ object Periods {
     }
   }
 
-  inline val NowhereCode = 0
-  final val Nowhere: Period = new Period(NowhereCode)
-
-  final val InitialPeriod: Period = Period(InitialRunId, FirstPhaseId)
-
-  final val InvalidPeriod: Period = Period(NoRunId, NoPhaseId)
-
   /** An ordinal number for compiler runs. First run has number 1. */
   type RunId = Int
   inline val NoRunId = 0
@@ -172,6 +170,11 @@ object Periods {
 
   /** The number of bits needed to encode a phase identifier. */
   inline val PhaseWidth = 7
-  inline val PhaseMask = (1 << PhaseWidth) - 1
+  private inline val PhaseMask = (1 << PhaseWidth) - 1
   inline val MaxPossiblePhaseId = PhaseMask
+
+  private inline val NowhereCode = 0
+  final val Nowhere: Period = new Period(NowhereCode)
+  final val InitialPeriod: Period = Period(InitialRunId, FirstPhaseId)
+  final val InvalidPeriod: Period = Period(NoRunId, NoPhaseId)
 }

compiler/src/dotty/tools/dotc/core/Phases.scala

@@ -12,7 +12,6 @@ import config.Printers.config
 import scala.collection.mutable.ListBuffer
 import dotty.tools.dotc.transform.MegaPhase.*
 import dotty.tools.dotc.transform.*
-import Periods.*
 import parsing.Parser
 import printing.XprintMode
 import typer.{TyperPhase, RefChecks}
@@ -64,7 +63,7 @@ object Phases {
       protected def run(using Context): Unit = unsupported("run")
       def transform(ref: SingleDenotation)(using Context): SingleDenotation =
         unsupported("transform")
-      override def lastPhaseId(using Context): Int = id
+      override def lastPhaseId(using Context): PhaseId = id
     }
 
     final def phasePlan: List[List[Phase]] = this.phasesPlan
@@ -153,7 +152,7 @@ object Phases {
       nextDenotTransformerId = new Array[Int](phases.length)
       denotTransformers = new Array[DenotTransformer](phases.length)
 
-      var phaseId = 0
+      var phaseId: PhaseId = 0
       def nextPhaseId = {
         phaseId += 1
         phaseId // starting from 1 as NoPhase is 0
@@ -260,7 +259,7 @@ object Phases {
     private var myGenBCodePhase: Phase = uninitialized
     private var myCheckCapturesPhase: Phase = uninitialized
 
-    private var myCheckCapturesPhaseId: Int = -2
+    private var myCheckCapturesPhaseId: PhaseId = -2
       // -1 means undefined, 0 means NoPhase, we make sure that we don't get a false hit
       // if ctx.phaseId is either of these.
 
@@ -292,7 +291,7 @@ object Phases {
     final def flattenPhase: Phase = myFlattenPhase
     final def genBCodePhase: Phase = myGenBCodePhase
     final def checkCapturesPhase: Phase = myCheckCapturesPhase
-    final def checkCapturesPhaseId: Int = myCheckCapturesPhaseId
+    final def checkCapturesPhaseId: PhaseId = myCheckCapturesPhaseId
 
     private def setSpecificPhases() = {
       def phaseOfClass(pclass: Class[?]) = phases.find(pclass.isInstance).getOrElse(NoPhase)
@@ -461,7 +460,7 @@ object Phases {
      */
     def printingContext(ctx: Context): Context = ctx
 
-    private var myPeriod: Period = Periods.InvalidPeriod
+    private var myPeriod: Period = InvalidPeriod
     private var myBase: ContextBase = uninitialized
     private var myErasedTypes = false
     private var myFlatClasses = false
@@ -477,29 +476,29 @@ object Phases {
      * is reserved for NoPhase and the first real phase is at position 1.
      * -1 if the phase is not installed in the context.
      */
-    def id: Int = myPeriod.firstPhaseId
+    def id: PhaseId = myPeriod.firstPhaseId
 
     def period: Period = myPeriod
-    def start: Int = myPeriod.firstPhaseId
-    def end: Periods.PhaseId = myPeriod.lastPhaseId
+    def start: PhaseId = myPeriod.firstPhaseId
+    def end: PhaseId = myPeriod.lastPhaseId
 
     final def erasedTypes: Boolean = myErasedTypes   // Phase is after erasure
     final def flatClasses: Boolean = myFlatClasses   // Phase is after flatten
     final def refChecked: Boolean = myRefChecked     // Phase is after RefChecks
     final def lambdaLifted: Boolean = myLambdaLifted // Phase is after LambdaLift
     final def patternTranslated: Boolean = myPatternTranslated // Phase is after PatternMatcher
 
-    final def sameMembersStartId: Int = mySameMembersStartId
+    final def sameMembersStartId: PhaseId = mySameMembersStartId
       // id of first phase where all symbols are guaranteed to have the same members as in this phase
-    final def sameParentsStartId: Int = mySameParentsStartId
+    final def sameParentsStartId: PhaseId = mySameParentsStartId
       // id of first phase where all symbols are guaranteed to have the same parents as in this phase
-    final def sameBaseTypesStartId: Int = mySameBaseTypesStartId
+    final def sameBaseTypesStartId: PhaseId = mySameBaseTypesStartId
       // id of first phase where all symbols are guaranteed to have the same base tpyes as in this phase
 
-    protected[Phases] def init(base: ContextBase, start: Int, end: Int): Unit = {
+    protected[Phases] def init(base: ContextBase, start: PhaseId, end: PhaseId): Unit = {
       if (start >= FirstPhaseId)
-        assert(myPeriod == Periods.InvalidPeriod, s"phase $this has already been used once; cannot be reused")
-      assert(start <= Periods.MaxPossiblePhaseId, s"Too many phases, Period bits overflow")
+        assert(myPeriod == InvalidPeriod, s"phase $this has already been used once; cannot be reused")
+      assert(start <= MaxPossiblePhaseId, s"Too many phases, Period bits overflow")
       myBase = base
       myPeriod = Period(NoRunId, start, end)
       myErasedTypes  = prev.getClass == classOf[Erasure]    || prev.erasedTypes
@@ -512,7 +511,7 @@ object Phases {
       mySameBaseTypesStartId = if (changesBaseTypes) id else prev.sameBaseTypesStartId
     }
 
-    protected[Phases] def init(base: ContextBase, id: Int): Unit = init(base, id, id)
+    protected[Phases] def init(base: ContextBase, id: PhaseId): Unit = init(base, id, id)
 
     final def <=(that: Phase): Boolean =
       exists && id <= that.id
@@ -585,7 +584,7 @@ object Phases {
   def flattenPhase(using Context): Phase                = ctx.base.flattenPhase
   def genBCodePhase(using Context): Phase               = ctx.base.genBCodePhase
   def checkCapturesPhase(using Context): Phase          = ctx.base.checkCapturesPhase
-  def checkCapturesPhaseId(using Context): Int          = ctx.base.checkCapturesPhaseId
+  def checkCapturesPhaseId(using Context): PhaseId      = ctx.base.checkCapturesPhaseId
 
   def unfusedPhases(using Context): Array[Phase] = ctx.base.phases
 

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -2123,7 +2123,7 @@ object SymDenotations {
       if (proceedWithEnter(sym, mscope)) {
         enterNoReplace(sym, mscope)
         val nxt = this.nextInRun
-        if (nxt.validFor.code > this.validFor.code)
+        if (nxt.validFor > this.validFor)
           this.nextInRun.asSymDenotation.asClass.enter(sym)
       }
     }
@@ -2965,7 +2965,7 @@ object SymDenotations {
     }
 
     def isValidAt(phase: Phase)(using Context) =
-      checkedPeriod.code == ctx.period.code ||
+      checkedPeriod == ctx.period ||
         createdAt.runId == ctx.runId &&
         createdAt.phaseId < unfusedPhases.length &&
         sameGroup(unfusedPhases(createdAt.phaseId), phase) &&

compiler/src/dotty/tools/dotc/core/Symbols.scala

@@ -105,7 +105,7 @@ object Symbols extends SymUtils {
     /** The current denotation of this symbol */
     final def denot(using Context): SymDenotation = {
       util.Stats.record("Symbol.denot")
-      if checkedPeriod.code == ctx.period.code then lastDenot
+      if checkedPeriod == ctx.period then lastDenot
       else computeDenot(lastDenot)
     }