From dc1fc60ca033e85f637febda77b38d09ce8c4658 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 9 Sep 2023 12:26:22 +0200 Subject: [PATCH 1/6] Change global enabling scheme for cc The aim is to have an efficient test whether a phase or denot transformer should be run. --- compiler/src/dotty/tools/dotc/Run.scala | 6 +++--- .../src/dotty/tools/dotc/cc/CheckCaptures.scala | 5 +++-- .../src/dotty/tools/dotc/config/Feature.scala | 6 +++--- compiler/src/dotty/tools/dotc/core/Phases.scala | 15 +++++++++++---- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/Run.scala b/compiler/src/dotty/tools/dotc/Run.scala index 9aaf12da3dcc..438561d15ada 100644 --- a/compiler/src/dotty/tools/dotc/Run.scala +++ b/compiler/src/dotty/tools/dotc/Run.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index d5bd8522ca92..4c7de4a176f7 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -29,7 +29,7 @@ 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. @@ -190,7 +190,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) diff --git a/compiler/src/dotty/tools/dotc/config/Feature.scala b/compiler/src/dotty/tools/dotc/config/Feature.scala index 5bcc139326f9..b64d7016e913 100644 --- a/compiler/src/dotty/tools/dotc/config/Feature.scala +++ b/compiler/src/dotty/tools/dotc/config/Feature.scala @@ -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) @@ -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 diff --git a/compiler/src/dotty/tools/dotc/core/Phases.scala b/compiler/src/dotty/tools/dotc/core/Phases.scala index 3fc7238cdd82..e62299ab8b76 100644 --- a/compiler/src/dotty/tools/dotc/core/Phases.scala +++ b/compiler/src/dotty/tools/dotc/core/Phases.scala @@ -299,6 +299,14 @@ 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. @@ -306,6 +314,9 @@ object Phases { // 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. */ @@ -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 From 38cd627b9b7d4f4cfcc556cc2046a730a255c877 Mon Sep 17 00:00:00 2001 From: odersky Date: Sat, 9 Sep 2023 12:53:26 +0200 Subject: [PATCH 2/6] A more robust scheme for resetting denotations after Recheck 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 effectivly only symbols touched by the Rechecker are transformed and reset again afterwards. --- .../dotty/tools/dotc/cc/CheckCaptures.scala | 15 ++++++---- compiler/src/dotty/tools/dotc/cc/Setup.scala | 2 +- .../tools/dotc/transform/PreRecheck.scala | 2 ++ .../dotty/tools/dotc/transform/Recheck.scala | 30 +++++++++---------- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index 4c7de4a176f7..6538cc1545df 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -34,14 +34,17 @@ object CheckCaptures: /** - 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, toCC = true) + else sym + else sym end Pre enum EnvKind: diff --git a/compiler/src/dotty/tools/dotc/cc/Setup.scala b/compiler/src/dotty/tools/dotc/cc/Setup.scala index adaa7219d68b..e18c5e559aba 100644 --- a/compiler/src/dotty/tools/dotc/cc/Setup.scala +++ b/compiler/src/dotty/tools/dotc/cc/Setup.scala @@ -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 _ => diff --git a/compiler/src/dotty/tools/dotc/transform/PreRecheck.scala b/compiler/src/dotty/tools/dotc/transform/PreRecheck.scala index db9e28d7aad7..ba60d3b97adc 100644 --- a/compiler/src/dotty/tools/dotc/transform/PreRecheck.scala +++ b/compiler/src/dotty/tools/dotc/transform/PreRecheck.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/transform/Recheck.scala b/compiler/src/dotty/tools/dotc/transform/Recheck.scala index 2456e4011367..d8db460b06a8 100644 --- a/compiler/src/dotty/tools/dotc/transform/Recheck.scala +++ b/compiler/src/dotty/tools/dotc/transform/Recheck.scala @@ -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 @@ -158,16 +151,20 @@ 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) + 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 */ @@ -197,6 +194,7 @@ abstract class Recheck extends Phase, SymTransformer: def reset()(using Context): Unit = for (ref, mbr) <- prevSelDenots.iterator do ref.withDenot(mbr) + preRecheckPhase /** Constant-folded rechecked type `tp` of tree `tree` */ protected def constFold(tree: Tree, tp: Type)(using Context): Type = From c325428b1e93f07c9a0f7b7ca19eb957a65da600 Mon Sep 17 00:00:00 2001 From: odersky Date: Sun, 10 Sep 2023 13:09:00 +0200 Subject: [PATCH 3/6] Simplify Synthetics.transform No backward mapping is necessary anymore. # Conflicts: # compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala --- .../dotty/tools/dotc/cc/CheckCaptures.scala | 8 +- .../src/dotty/tools/dotc/cc/Synthetics.scala | 79 +++++-------------- 2 files changed, 21 insertions(+), 66 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala index 6538cc1545df..09f55dc35583 100644 --- a/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala +++ b/compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala @@ -42,7 +42,7 @@ object CheckCaptures: 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) + Synthetics.transform(sym) else sym else sym end Pre @@ -203,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): diff --git a/compiler/src/dotty/tools/dotc/cc/Synthetics.scala b/compiler/src/dotty/tools/dotc/cc/Synthetics.scala index 1e7c8d641238..1509fd838265 100644 --- a/compiler/src/dotty/tools/dotc/cc/Synthetics.scala +++ b/compiler/src/dotty/tools/dotc/cc/Synthetics.scala @@ -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: @@ -92,19 +92,7 @@ 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 => @@ -112,11 +100,10 @@ object Synthetics: 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) { @@ -126,32 +113,19 @@ 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)) @@ -159,16 +133,9 @@ object Synthetics: 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 @@ -179,18 +146,10 @@ 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) => @@ -198,7 +157,7 @@ object Synthetics: 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 => From 8e6986176157f6320341a55a74a4ceff6840147a Mon Sep 17 00:00:00 2001 From: odersky Date: Thu, 14 Sep 2023 11:36:57 +0200 Subject: [PATCH 4/6] Print TypeError stacktraces on -Ydebug-error or -Ydebug-type-error --- compiler/src/dotty/tools/dotc/core/TypeErrors.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 1dcd2301b1a7..87e4ba923e58 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -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 From 56eb2d8291a95b15d49cc4099c696f409a7273f3 Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 18 Sep 2023 22:09:13 +0200 Subject: [PATCH 5/6] Update compiler/src/dotty/tools/dotc/transform/Recheck.scala --- compiler/src/dotty/tools/dotc/transform/Recheck.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Recheck.scala b/compiler/src/dotty/tools/dotc/transform/Recheck.scala index d8db460b06a8..ae5f65f15123 100644 --- a/compiler/src/dotty/tools/dotc/transform/Recheck.scala +++ b/compiler/src/dotty/tools/dotc/transform/Recheck.scala @@ -194,7 +194,6 @@ abstract class Recheck extends Phase, SymTransformer: def reset()(using Context): Unit = for (ref, mbr) <- prevSelDenots.iterator do ref.withDenot(mbr) - preRecheckPhase /** Constant-folded rechecked type `tp` of tree `tree` */ protected def constFold(tree: Tree, tp: Type)(using Context): Type = From ddc4eb85817979c4691edcb8cbaff8a55c965945 Mon Sep 17 00:00:00 2001 From: odersky Date: Wed, 20 Sep 2023 12:28:20 +0200 Subject: [PATCH 6/6] Fix bug when restoring denotations in Recheck. Need to copy the denotation, since denotations come with next pointers which would get scrambled otherwise. The bug was observed when compiling stdlib under new capture checking implementation. # Conflicts: # compiler/src/dotty/tools/dotc/transform/Recheck.scala --- compiler/src/dotty/tools/dotc/transform/Recheck.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/Recheck.scala b/compiler/src/dotty/tools/dotc/transform/Recheck.scala index ae5f65f15123..306ca2b0eb9c 100644 --- a/compiler/src/dotty/tools/dotc/transform/Recheck.scala +++ b/compiler/src/dotty/tools/dotc/transform/Recheck.scala @@ -153,7 +153,8 @@ abstract class Recheck extends Phase, SymTransformer: /** Change any `ResetPrivate` flags back to `Private` */ def transformSym(symd: SymDenotation)(using Context): SymDenotation = val sym = symd.symbol - if sym.isUpdatedAfter(preRecheckPhase) then atPhase(preRecheckPhase)(sym.denot) + if sym.isUpdatedAfter(preRecheckPhase) + then atPhase(preRecheckPhase)(sym.denot.copySymDenotation()) else symd def run(using Context): Unit =