Skip to content

Commit

Permalink
A more robust scheme for resetting denotations after Recheck (#18534)
Browse files Browse the repository at this point in the history
The new scheme works also for arbitrary denotation changes in
PreRecheck.

Furthermore, recheck denot transformers are not run after Recheck has
ended. This
means that effectively only symbols touched by the Rechecker are
transformed and
reset again afterwards.
  • Loading branch information
odersky authored Oct 1, 2023
2 parents ac5fcfb + ddc4eb8 commit 09ea77e
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 101 deletions.
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,10 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
*/
var pureFunsImportEncountered = false

/** Will be set to true if any of the compiled compilation units contains
* a captureChecking language import.
/** Will be set to true if experimental.captureChecking is enabled
* or any of the compiled compilation units contains a captureChecking language import.
*/
var ccImportEncountered = false
var ccEnabledSomewhere = Feature.enabledBySetting(Feature.captureChecking)(using ictx)

private var myEnrichedErrorMessage = false

Expand Down
26 changes: 13 additions & 13 deletions compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,22 @@ object CheckCaptures:

class Pre extends PreRecheck, SymTransformer:

override def isEnabled(using Context) = true
override def isRunnable(using Context) = super.isRunnable && Feature.ccEnabledSomewhere

/** - Reset `private` flags of parameter accessors so that we can refine them
* in Setup if they have non-empty capture sets.
* - Special handling of some symbols defined for case classes.
* Enabled only until recheck is finished, and provided some compilation unit
* is CC-enabled.
*/
def transformSym(sym: SymDenotation)(using Context): SymDenotation =
if sym.isAllOf(PrivateParamAccessor) && !sym.hasAnnotation(defn.ConstructorOnlyAnnot) then
sym.copySymDenotation(initFlags = sym.flags &~ Private | Recheck.ResetPrivate)
else if Synthetics.needsTransform(sym) then
Synthetics.transform(sym, toCC = true)
else
sym
if !pastRecheck && Feature.ccEnabledSomewhere then
if sym.isAllOf(PrivateParamAccessor) && !sym.hasAnnotation(defn.ConstructorOnlyAnnot) then
sym.copySymDenotation(initFlags = sym.flags &~ Private | Recheck.ResetPrivate)
else if Synthetics.needsTransform(sym) then
Synthetics.transform(sym)
else sym
else sym
end Pre

enum EnvKind:
Expand Down Expand Up @@ -190,7 +193,8 @@ class CheckCaptures extends Recheck, SymTransformer:
import CheckCaptures.*

def phaseName: String = "cc"
override def isEnabled(using Context) = true

override def isRunnable(using Context) = super.isRunnable && Feature.ccEnabledSomewhere

def newRechecker()(using Context) = CaptureChecker(ctx)

Expand All @@ -199,11 +203,7 @@ class CheckCaptures extends Recheck, SymTransformer:
override def run(using Context): Unit =
if Feature.ccEnabled then
super.run

override def transformSym(sym: SymDenotation)(using Context): SymDenotation =
if Synthetics.needsTransform(sym) then Synthetics.transform(sym, toCC = false)
else super.transformSym(sym)


override def printingContext(ctx: Context) = ctx.withProperty(ccStateKey, Some(new CCState))

class CaptureChecker(ictx: Context) extends Rechecker(ictx):
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/cc/Setup.scala
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ extends tpd.TreeTraverser:

/** Update info of `sym` for CheckCaptures phase only */
private def updateInfo(sym: Symbol, info: Type)(using Context) =
sym.updateInfoBetween(preRecheckPhase, thisPhase, info, newOwnerFor(sym))
sym.updateInfo(preRecheckPhase, info, newOwnerFor(sym))
sym.namedType match
case ref: CaptureRef => ref.invalidateCaches()
case _ =>
Expand Down
79 changes: 19 additions & 60 deletions compiler/src/dotty/tools/dotc/cc/Synthetics.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ object Synthetics:
* @param sym The method to transform @pre needsTransform(sym) must hold.
* @param toCC Whether to transform the type to capture checking or back.
*/
def transform(sym: SymDenotation, toCC: Boolean)(using Context): SymDenotation =
def transform(sym: SymDenotation)(using Context): SymDenotation =

/** Add capture dependencies to the type of the `apply` or `copy` method of a case class.
* An apply method in a case class like this:
Expand Down Expand Up @@ -92,31 +92,18 @@ object Synthetics:
case _ =>
info

/** Drop capture dependencies from the type of `apply` or `copy` method of a case class */
def dropCaptureDeps(tp: Type): Type = tp match
case tp: MethodOrPoly =>
tp.derivedLambdaType(resType = dropCaptureDeps(tp.resType))
case CapturingType(parent, _) =>
dropCaptureDeps(parent)
case RefinedType(parent, _, _) =>
dropCaptureDeps(parent)
case _ =>
tp

/** Add capture information to the type of the default getter of a case class copy method
* if toCC = true, or remove the added info again if toCC = false.
*/
def transformDefaultGetterCaptures(info: Type, owner: Symbol, idx: Int)(using Context): Type = info match
case info: MethodOrPoly =>
info.derivedLambdaType(resType = transformDefaultGetterCaptures(info.resType, owner, idx))
case info: ExprType =>
info.derivedExprType(transformDefaultGetterCaptures(info.resType, owner, idx))
case EventuallyCapturingType(parent, _) =>
if toCC then transformDefaultGetterCaptures(parent, owner, idx)
else parent
transformDefaultGetterCaptures(parent, owner, idx)
case info @ AnnotatedType(parent, annot) =>
info.derivedAnnotatedType(transformDefaultGetterCaptures(parent, owner, idx), annot)
case _ if toCC && idx < owner.asClass.paramGetters.length =>
case _ if idx < owner.asClass.paramGetters.length =>
val param = owner.asClass.paramGetters(idx)
val pinfo = param.info
atPhase(ctx.phase.next) {
Expand All @@ -126,49 +113,29 @@ object Synthetics:
case _ =>
info

/** Augment an unapply of type `(x: C): D` to `(x: C^{cap}): D^{x}` if toCC is true,
* or remove the added capture sets again if toCC = false.
*/
/** Augment an unapply of type `(x: C): D` to `(x: C^{cap}): D^{x}` */
def transformUnapplyCaptures(info: Type)(using Context): Type = info match
case info: MethodType =>
if toCC then
val paramInfo :: Nil = info.paramInfos: @unchecked
val newParamInfo = CapturingType(paramInfo, CaptureSet.universal)
val trackedParam = info.paramRefs.head
def newResult(tp: Type): Type = tp match
case tp: MethodOrPoly =>
tp.derivedLambdaType(resType = newResult(tp.resType))
case _ =>
CapturingType(tp, CaptureSet(trackedParam))
info.derivedLambdaType(paramInfos = newParamInfo :: Nil, resType = newResult(info.resType))
.showing(i"augment unapply type $info to $result", capt)
else info.paramInfos match
case CapturingType(oldParamInfo, _) :: Nil =>
def oldResult(tp: Type): Type = tp match
case tp: MethodOrPoly =>
tp.derivedLambdaType(resType = oldResult(tp.resType))
case CapturingType(tp, _) =>
tp
info.derivedLambdaType(paramInfos = oldParamInfo :: Nil, resType = oldResult(info.resType))
val paramInfo :: Nil = info.paramInfos: @unchecked
val newParamInfo = CapturingType(paramInfo, CaptureSet.universal)
val trackedParam = info.paramRefs.head
def newResult(tp: Type): Type = tp match
case tp: MethodOrPoly =>
tp.derivedLambdaType(resType = newResult(tp.resType))
case _ =>
info
CapturingType(tp, CaptureSet(trackedParam))
info.derivedLambdaType(paramInfos = newParamInfo :: Nil, resType = newResult(info.resType))
.showing(i"augment unapply type $info to $result", capt)
case info: PolyType =>
info.derivedLambdaType(resType = transformUnapplyCaptures(info.resType))

def transformComposeCaptures(symd: SymDenotation) =
val (pt: PolyType) = symd.info: @unchecked
val (mt: MethodType) = pt.resType: @unchecked
val (enclThis: ThisType) = symd.owner.thisType: @unchecked
val mt1 =
if toCC then
MethodType(mt.paramNames)(
mt1 => mt.paramInfos.map(_.capturing(CaptureSet.universal)),
mt1 => CapturingType(mt.resType, CaptureSet(enclThis, mt1.paramRefs.head)))
else
MethodType(mt.paramNames)(
mt1 => mt.paramInfos.map(_.stripCapturing),
mt1 => mt.resType.stripCapturing)
pt.derivedLambdaType(resType = mt1)
pt.derivedLambdaType(resType = MethodType(mt.paramNames)(
mt1 => mt.paramInfos.map(_.capturing(CaptureSet.universal)),
mt1 => CapturingType(mt.resType, CaptureSet(enclThis, mt1.paramRefs.head))))

def transformCurriedTupledCaptures(symd: SymDenotation) =
val (et: ExprType) = symd.info: @unchecked
Expand All @@ -179,26 +146,18 @@ object Synthetics:
defn.FunctionOf(args, mapFinalResult(res, f), isContextual)
else
f(tp)
val resType1 =
if toCC then
mapFinalResult(et.resType, CapturingType(_, CaptureSet(enclThis)))
else
et.resType.stripCapturing
ExprType(resType1)
ExprType(mapFinalResult(et.resType, CapturingType(_, CaptureSet(enclThis))))

def transformCompareCaptures =
if toCC then
MethodType(defn.ObjectType.capturing(CaptureSet.universal) :: Nil, defn.BooleanType)
else
defn.methOfAnyRef(defn.BooleanType)
MethodType(defn.ObjectType.capturing(CaptureSet.universal) :: Nil, defn.BooleanType)

sym.copySymDenotation(info = sym.name match
case DefaultGetterName(nme.copy, n) =>
transformDefaultGetterCaptures(sym.info, sym.owner, n)
case nme.unapply =>
transformUnapplyCaptures(sym.info)
case nme.apply | nme.copy =>
if toCC then addCaptureDeps(sym.info) else dropCaptureDeps(sym.info)
addCaptureDeps(sym.info)
case nme.andThen | nme.compose =>
transformComposeCaptures(sym)
case nme.curried | nme.tupled =>
Expand Down
6 changes: 3 additions & 3 deletions compiler/src/dotty/tools/dotc/config/Feature.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ object Feature:

/** Is captureChecking enabled for any of the currently compiled compilation units? */
def ccEnabledSomewhere(using Context) =
enabledBySetting(captureChecking)
|| ctx.run != null && ctx.run.nn.ccImportEncountered
if ctx.run != null then ctx.run.nn.ccEnabledSomewhere
else enabledBySetting(captureChecking)

def sourceVersionSetting(using Context): SourceVersion =
SourceVersion.valueOf(ctx.settings.source.value)
Expand Down Expand Up @@ -174,7 +174,7 @@ object Feature:
true
else if fullFeatureName == captureChecking then
ctx.compilationUnit.needsCaptureChecking = true
if ctx.run != null then ctx.run.nn.ccImportEncountered = true
if ctx.run != null then ctx.run.nn.ccEnabledSomewhere = true
true
else
false
Expand Down
15 changes: 11 additions & 4 deletions compiler/src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,24 @@ object Phases {
*/
def phaseName: String

/** This property is queried when phases are first assembled.
* If it is false, the phase will be dropped from the set of phases to traverse.
*/
def isEnabled(using Context): Boolean = true

/** This property is queried before a phase is run.
* If it is false, the phase is skipped.
*/
def isRunnable(using Context): Boolean =
!ctx.reporter.hasErrors
// TODO: This might test an unintended condition.
// To find out whether any errors have been reported during this
// run one calls `errorsReported`, not `hasErrors`.
// But maybe changing this would prevent useful phases from running?

/** True for all phases except NoPhase */
def exists: Boolean = true

/** If set, allow missing or superfluous arguments in applications
* and type applications.
*/
Expand Down Expand Up @@ -360,10 +371,6 @@ object Phases {
/** Can this transform change the base types of a type? */
def changesBaseTypes: Boolean = changesParents

def isEnabled(using Context): Boolean = true

def exists: Boolean = true

def initContext(ctx: FreshContext): Unit = ()

/** A hook that allows to transform the usual context passed to the function
Expand Down
5 changes: 4 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ abstract class TypeError(using creationContext: Context) extends Exception(""):
* This is expensive and only useful for debugging purposes.
*/
def computeStackTrace: Boolean =
ctx.debug || (cyclicErrors != noPrinter && this.isInstanceOf[CyclicReference] && !(ctx.mode is Mode.CheckCyclic))
ctx.debug
|| (cyclicErrors != noPrinter && this.isInstanceOf[CyclicReference] && !(ctx.mode is Mode.CheckCyclic))
|| ctx.settings.YdebugTypeError.value
|| ctx.settings.YdebugError.value

override def fillInStackTrace(): Throwable =
if computeStackTrace then super.fillInStackTrace().nn
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/transform/PreRecheck.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ abstract class PreRecheck extends Phase, DenotTransformer:

override def changesBaseTypes: Boolean = true

var pastRecheck = false

def run(using Context): Unit = ()

override def isCheckable = false
30 changes: 14 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/Recheck.scala
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,19 @@ object Recheck:

extension (sym: Symbol)

/** Update symbol's info to newInfo from prevPhase.next to lastPhase.
/** Update symbol's info to newInfo after `prevPhase`.
* Also update owner to newOwnerOrNull if it is not null.
* Reset to previous info and owner for phases after lastPhase.
* The update is valid until after Recheck. After that the symbol's denotation
* is reset to what it was before PreRecheck.
*/
def updateInfoBetween(prevPhase: DenotTransformer, lastPhase: DenotTransformer, newInfo: Type, newOwnerOrNull: Symbol | Null = null)(using Context): Unit =
def updateInfo(prevPhase: DenotTransformer, newInfo: Type, newOwnerOrNull: Symbol | Null = null)(using Context): Unit =
val newOwner = if newOwnerOrNull == null then sym.owner else newOwnerOrNull
if (sym.info ne newInfo) || (sym.owner ne newOwner) then
val flags = sym.flags
sym.copySymDenotation(
initFlags =
if flags.isAllOf(ResetPrivateParamAccessor)
then flags &~ ResetPrivate | Private
else flags
).installAfter(lastPhase) // reset
sym.copySymDenotation(
owner = newOwner,
info = newInfo,
initFlags =
if newInfo.isInstanceOf[LazyType] then flags &~ Touched
else flags
initFlags = if newInfo.isInstanceOf[LazyType] then flags &~ Touched else flags
).installAfter(prevPhase)

/** Does symbol have a new denotation valid from phase.next that is different
Expand Down Expand Up @@ -158,16 +151,21 @@ abstract class Recheck extends Phase, SymTransformer:
// One failing test is pos/i583a.scala

/** Change any `ResetPrivate` flags back to `Private` */
def transformSym(sym: SymDenotation)(using Context): SymDenotation =
if sym.isAllOf(Recheck.ResetPrivateParamAccessor) then
sym.copySymDenotation(initFlags = sym.flags &~ Recheck.ResetPrivate | Private)
else sym
def transformSym(symd: SymDenotation)(using Context): SymDenotation =
val sym = symd.symbol
if sym.isUpdatedAfter(preRecheckPhase)
then atPhase(preRecheckPhase)(sym.denot.copySymDenotation())
else symd

def run(using Context): Unit =
val rechecker = newRechecker()
rechecker.checkUnit(ctx.compilationUnit)
rechecker.reset()

override def runOn(units: List[CompilationUnit])(using runCtx: Context): List[CompilationUnit] =
try super.runOn(units)
finally preRecheckPhase.pastRecheck = true

def newRechecker()(using Context): Rechecker

/** The typechecker pass */
Expand Down

0 comments on commit 09ea77e

Please sign in to comment.