Skip to content

Fixes to make dotc compile with capture checking #16254

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 37 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1d2e16e
Bugfix: generate same kind of MethodType when capture checking anonym…
odersky Nov 6, 2022
5496ef1
Bugfix: Avoid crash in cc/Synthetics
odersky Nov 6, 2022
65d8b19
Performance: Run capture checking transformers only if cc is enabled …
odersky Nov 6, 2022
2066efa
Enhancement: Revert automatic boxing of universal variable initializers
odersky Nov 6, 2022
f17fff9
Enhancement: Special treatment of arguments of `asInstanceOf`
odersky Nov 6, 2022
feb4c7c
Enhancement: Assume special capturing types for `eq` and `ne`
odersky Nov 6, 2022
96643ef
Bugfix: Allow all private definitions to have inferred types
odersky Nov 6, 2022
824580e
Bugfix: Avoid spurious check in RefChecks
odersky Nov 6, 2022
7740fb7
Bugfix: More lenient definition when an inferred type is OK for visib…
odersky Nov 6, 2022
4b98155
Enhancement: Don't count @constructorOnly parameters towards the self…
odersky Nov 6, 2022
b21867f
Enhancement: More lenient check for inferred self types
odersky Nov 6, 2022
4b97e1d
Enhancement: Add missing case for comparison of capturing types.
odersky Nov 6, 2022
7f0e259
Enhancement: Force all exception classes to be pure
odersky Nov 6, 2022
3964599
Tweak: Don't refine parameters of Java classes
odersky Nov 6, 2022
2ba6289
Tweak: Tweak rechecking of returns
odersky Nov 6, 2022
d157daa
Bugfix: Refine canWidenAbstract criterion
odersky Nov 6, 2022
18b8ff4
Tweak: Widen skolem types before conformity checks
odersky Nov 6, 2022
1b15fa6
Bugfix: Relax experimental inheritance criterion
odersky Nov 6, 2022
055beab
Enhancement: Introduce caps.Pure trait
odersky Nov 6, 2022
f194098
Tweak: Make root addition handler take a context
odersky Nov 6, 2022
f6e1c03
Bugfix: Fix setup of overriding symbols
odersky Nov 6, 2022
65477c5
Bugfix: Make another map an IdempotentCaptRefMap
odersky Nov 6, 2022
594aa1a
Bugfix: Fix handling for call-by-name arguments of applied types
odersky Nov 6, 2022
f8c4482
Bugfix: Make sure to restore anonymous function infos
odersky Nov 6, 2022
10c657c
Tweak: Exclude default getters from "must be explicitly defined" requ…
odersky Nov 7, 2022
c2086df
Enhancement: Treat Any as a top type for comparisons.
odersky Nov 7, 2022
ff5726c
Enhancement: Implement bounds checking
odersky Nov 7, 2022
25a4246
Enhancement: Take purity of classes into account when capture checking
odersky Nov 7, 2022
f079fe5
Enhancement: Add unsafeBoxFunArg operation.
odersky Nov 7, 2022
96bbc1d
Bugfix: Restore cached denotations of NamedTypes to their value befor…
odersky Nov 7, 2022
78dc699
Bugfix: Maintain inline context when rechecking
odersky Nov 7, 2022
2d5e981
Tweak: Avoid type ascription in uncheckedNN
odersky Nov 7, 2022
0381e2e
Enhancement: Move unsafe box/unbox ops into separate caps.unsafe module
odersky Nov 7, 2022
5d7b043
Tweak: Print Ranges like regular types
odersky Nov 7, 2022
f07dc95
Enhancement: Generalize handling of exceptions to all pure base classes
odersky Nov 7, 2022
16b5f4e
Fix typos
odersky Nov 10, 2022
dc4d6c9
Add doc comment to postCheck
odersky Nov 10, 2022
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
13 changes: 12 additions & 1 deletion compiler/src/dotty/tools/dotc/Run.scala
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,15 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
private var finalizeActions = mutable.ListBuffer[() => Unit]()

/** Will be set to true if any of the compiled compilation units contains
* a pureFunctions or captureChecking language import.
* a pureFunctions language import.
*/
var pureFunsImportEncountered = false

/** Will be set to true if any of the compiled compilation units contains
* a captureChecking language import.
*/
var ccImportEncountered = false

def compile(files: List[AbstractFile]): Unit =
try
val sources = files.map(runContext.getSource(_))
Expand Down Expand Up @@ -229,6 +234,7 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
def runPhases(using Context) = {
var lastPrintedTree: PrintedTree = NoPrintedTree
val profiler = ctx.profiler
var phasesWereAdjusted = false

for (phase <- ctx.base.allPhases)
if (phase.isRunnable)
Expand All @@ -247,6 +253,11 @@ class Run(comp: Compiler, ictx: Context) extends ImplicitRunInfo with Constraint
Stats.record(s"retained typed trees at end of $phase", unit.tpdTree.treeSize)
ctx.typerState.gc()
}
if !phasesWereAdjusted then
phasesWereAdjusted = true
if !Feature.ccEnabledSomewhere then
ctx.base.unlinkPhaseAsDenotTransformer(Phases.checkCapturesPhase.prev)
ctx.base.unlinkPhaseAsDenotTransformer(Phases.checkCapturesPhase)

profiler.finished()
}
Expand Down
41 changes: 41 additions & 0 deletions compiler/src/dotty/tools/dotc/cc/CaptureOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,49 @@ extension (tp: Type)
case CapturingType(_, _) => true
case _ => false

/** Is type known to be always pure by its class structure,
* so that adding a capture set to it would not make sense?
*/
def isAlwaysPure(using Context): Boolean = tp.dealias match
case tp: (TypeRef | AppliedType) =>
val sym = tp.typeSymbol
if sym.isClass then sym.isPureClass
else tp.superType.isAlwaysPure
case CapturingType(parent, refs) =>
parent.isAlwaysPure || refs.isAlwaysEmpty
case tp: TypeProxy =>
tp.superType.isAlwaysPure
case tp: AndType =>
tp.tp1.isAlwaysPure || tp.tp2.isAlwaysPure
case tp: OrType =>
tp.tp1.isAlwaysPure && tp.tp2.isAlwaysPure
case _ =>
false

extension (cls: ClassSymbol)

def pureBaseClass(using Context): Option[Symbol] =
cls.baseClasses.find(bc =>
defn.pureBaseClasses.contains(bc)
|| {
val selfType = bc.givenSelfType
selfType.exists && selfType.captureSet.isAlwaysEmpty
})

extension (sym: Symbol)

/** A class is pure if:
* - one its base types has an explicitly declared self type with an empty capture set
* - or it is a value class
* - or it is an exception
* - or it is one of Nothing, Null, or String
*/
def isPureClass(using Context): Boolean = sym match
case cls: ClassSymbol =>
cls.pureBaseClass.isDefined || defn.pureSimpleClasses.contains(cls)
case _ =>
false

/** Does this symbol allow results carrying the universal capability?
* Currently this is true only for function type applies (since their
* results are unboxed) and `erasedValue` since this function is magic in
Expand Down
10 changes: 5 additions & 5 deletions compiler/src/dotty/tools/dotc/cc/CaptureSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ sealed abstract class CaptureSet extends Showable:
map(Substituters.SubstParamsMap(tl, to))

/** Invoke handler if this set has (or later aquires) the root capability `*` */
def disallowRootCapability(handler: () => Unit)(using Context): this.type =
def disallowRootCapability(handler: () => Context ?=> Unit)(using Context): this.type =
if isUniversal then handler()
this

Expand Down Expand Up @@ -373,7 +373,7 @@ object CaptureSet:
def isAlwaysEmpty = false

/** A handler to be invoked if the root reference `*` is added to this set */
var addRootHandler: () => Unit = () => ()
var rootAddedHandler: () => Context ?=> Unit = () => ()

var description: String = ""

Expand Down Expand Up @@ -404,7 +404,7 @@ object CaptureSet:
def addNewElems(newElems: Refs, origin: CaptureSet)(using Context, VarState): CompareResult =
if !isConst && recordElemsState() then
elems ++= newElems
if isUniversal then addRootHandler()
if isUniversal then rootAddedHandler()
// assert(id != 2 || elems.size != 2, this)
(CompareResult.OK /: deps) { (r, dep) =>
r.andAlso(dep.tryInclude(newElems, this))
Expand All @@ -421,8 +421,8 @@ object CaptureSet:
else
CompareResult.fail(this)

override def disallowRootCapability(handler: () => Unit)(using Context): this.type =
addRootHandler = handler
override def disallowRootCapability(handler: () => Context ?=> Unit)(using Context): this.type =
rootAddedHandler = handler
super.disallowRootCapability(handler)

private var computingApprox = false
Expand Down
119 changes: 80 additions & 39 deletions compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ import config.Printers.{capt, recheckr}
import config.{Config, Feature}
import ast.{tpd, untpd, Trees}
import Trees.*
import typer.RefChecks.{checkAllOverrides, checkParents}
import typer.RefChecks.{checkAllOverrides, checkSelfAgainstParents}
import typer.Checking.{checkBounds, checkAppliedTypesIn}
import util.{SimpleIdentitySet, EqHashMap, SrcPos}
import transform.SymUtils.*
import transform.{Recheck, PreRecheck}
import Recheck.*
import scala.collection.mutable
import CaptureSet.{withCaptureSetsExplained, IdempotentCaptRefMap}
import StdNames.nme
import NameKinds.DefaultGetterName
import reporting.trace

/** The capture checker */
Expand Down Expand Up @@ -335,12 +337,21 @@ class CheckCaptures extends Recheck, SymTransformer:
override def recheckApply(tree: Apply, pt: Type)(using Context): Type =
val meth = tree.fun.symbol
includeCallCaptures(meth, tree.srcPos)
if meth == defn.Caps_unsafeBox || meth == defn.Caps_unsafeUnbox then
def mapArgUsing(f: Type => Type) =
val arg :: Nil = tree.args: @unchecked
val argType0 = recheckStart(arg, pt)
.forceBoxStatus(boxed = meth == defn.Caps_unsafeBox)
val argType0 = f(recheckStart(arg, pt))
val argType = super.recheckFinish(argType0, arg, pt)
super.recheckFinish(argType, tree, pt)

if meth == defn.Caps_unsafeBox then
mapArgUsing(_.forceBoxStatus(true))
else if meth == defn.Caps_unsafeUnbox then
mapArgUsing(_.forceBoxStatus(false))
else if meth == defn.Caps_unsafeBoxFunArg then
mapArgUsing {
case defn.FunctionOf(paramtpe :: Nil, restpe, isContectual, isErased) =>
defn.FunctionOf(paramtpe.forceBoxStatus(true) :: Nil, restpe, isContectual, isErased)
}
else
super.recheckApply(tree, pt) match
case appType @ CapturingType(appType1, refs) =>
Expand Down Expand Up @@ -432,7 +443,8 @@ class CheckCaptures extends Recheck, SymTransformer:
block match
case closureDef(mdef) =>
pt.dealias match
case defn.FunctionOf(ptformals, _, _, _) if ptformals.forall(_.captureSet.isAlwaysEmpty) =>
case defn.FunctionOf(ptformals, _, _, _)
if ptformals.nonEmpty && ptformals.forall(_.captureSet.isAlwaysEmpty) =>
// Redo setup of the anonymous function so that formal parameters don't
// get capture sets. This is important to avoid false widenings to `*`
// when taking the base type of the actual closures's dependent function
Expand All @@ -442,46 +454,32 @@ class CheckCaptures extends Recheck, SymTransformer:
// First, undo the previous setup which installed a completer for `meth`.
atPhase(preRecheckPhase.prev)(meth.denot.copySymDenotation())
.installAfter(preRecheckPhase)

// Next, update all parameter symbols to match expected formals
meth.paramSymss.head.lazyZip(ptformals).foreach { (psym, pformal) =>
psym.copySymDenotation(info = pformal).installAfter(preRecheckPhase)
psym.updateInfoBetween(preRecheckPhase, thisPhase, pformal.mapExprType)
}
// Next, update types of parameter ValDefs
mdef.paramss.head.lazyZip(ptformals).foreach { (param, pformal) =>
val ValDef(_, tpt, _) = param: @unchecked
tpt.rememberTypeAlways(pformal)
}
// Next, install a new completer reflecting the new parameters for the anonymous method
val mt = meth.info.asInstanceOf[MethodType]
val completer = new LazyType:
def complete(denot: SymDenotation)(using Context) =
denot.info = MethodType(ptformals, mdef.tpt.knownType)
denot.info = mt.companion(ptformals, mdef.tpt.knownType)
.showing(i"simplify info of $meth to $result", capt)
recheckDef(mdef, meth)
meth.copySymDenotation(info = completer, initFlags = meth.flags &~ Touched)
.installAfter(preRecheckPhase)
meth.updateInfoBetween(preRecheckPhase, thisPhase, completer)
case _ =>
case _ =>
super.recheckBlock(block, pt)

/** If `rhsProto` has `*` as its capture set, wrap `rhs` in a `unsafeBox`.
* Used to infer `unsafeBox` for expressions that get assigned to variables
* that have universal capture set.
*/
def maybeBox(rhs: Tree, rhsProto: Type)(using Context): Tree =
if rhsProto.captureSet.isUniversal then
ref(defn.Caps_unsafeBox).appliedToType(rhsProto).appliedTo(rhs)
else rhs

override def recheckAssign(tree: Assign)(using Context): Type =
val rhsProto = recheck(tree.lhs).widen
recheck(maybeBox(tree.rhs, rhsProto), rhsProto)
defn.UnitType

override def recheckValDef(tree: ValDef, sym: Symbol)(using Context): Unit =
try
if !sym.is(Module) then // Modules are checked by checking the module class
if sym.is(Mutable) then recheck(maybeBox(tree.rhs, sym.info), sym.info)
else super.recheckValDef(tree, sym)
super.recheckValDef(tree, sym)
finally
if !sym.is(Param) then
// Parameters with inferred types belong to anonymous methods. We need to wait
Expand All @@ -503,7 +501,8 @@ class CheckCaptures extends Recheck, SymTransformer:
/** Class-specific capture set relations:
* 1. The capture set of a class includes the capture sets of its parents.
* 2. The capture set of the self type of a class includes the capture set of the class.
* 3. The capture set of the self type of a class includes the capture set of every class parameter.
* 3. The capture set of the self type of a class includes the capture set of every class parameter,
* unless the parameter is marked @constructorOnly.
*/
override def recheckClassDef(tree: TypeDef, impl: Template, cls: ClassSymbol)(using Context): Type =
val saved = curEnv
Expand All @@ -515,7 +514,12 @@ class CheckCaptures extends Recheck, SymTransformer:
val thisSet = cls.classInfo.selfType.captureSet.withDescription(i"of the self type of $cls")
checkSubset(localSet, thisSet, tree.srcPos) // (2)
for param <- cls.paramGetters do
checkSubset(param.termRef.captureSet, thisSet, param.srcPos) // (3)
if !param.hasAnnotation(defn.ConstructorOnlyAnnot) then
checkSubset(param.termRef.captureSet, thisSet, param.srcPos) // (3)
for pureBase <- cls.pureBaseClass do
checkSubset(thisSet,
CaptureSet.empty.withDescription(i"of pure base class $pureBase"),
tree.srcPos)
super.recheckClassDef(tree, impl, cls)
finally
curEnv = saved
Expand Down Expand Up @@ -772,7 +776,8 @@ class CheckCaptures extends Recheck, SymTransformer:
// We can't box/unbox the universal capability. Leave `actual` as it is
// so we get an error in checkConforms. This tends to give better error
// messages than disallowing the root capability in `criticalSet`.
capt.println(i"cannot box/unbox $actual vs $expected")
if ctx.settings.YccDebug.value then
println(i"cannot box/unbox $actual vs $expected")
actual
else
// Disallow future addition of `*` to `criticalSet`.
Expand Down Expand Up @@ -845,13 +850,21 @@ class CheckCaptures extends Recheck, SymTransformer:
cls => !parentTrees(cls).exists(ptree => parentTrees.contains(ptree.tpe.classSymbol))
}
assert(roots.nonEmpty)
for root <- roots do
checkParents(root, parentTrees(root))
for case root: ClassSymbol <- roots do
checkSelfAgainstParents(root, root.baseClasses)
val selfType = root.asClass.classInfo.selfType
interpolator(startingVariance = -1).traverse(selfType)
if !root.isEffectivelySealed then
def matchesExplicitRefsInBaseClass(refs: CaptureSet, cls: ClassSymbol): Boolean =
cls.baseClasses.tail.exists { psym =>
val selfType = psym.asClass.givenSelfType
selfType.exists && selfType.captureSet.elems == refs.elems
}
selfType match
case CapturingType(_, refs: CaptureSet.Var) if !refs.isUniversal =>
case CapturingType(_, refs: CaptureSet.Var)
if !refs.isUniversal && !matchesExplicitRefsInBaseClass(refs, root) =>
// Forbid inferred self types unless they are already implied by an explicit
// self type in a parent.
report.error(
i"""$root needs an explicitly declared self type since its
|inferred self type $selfType
Expand All @@ -867,6 +880,7 @@ class CheckCaptures extends Recheck, SymTransformer:
* - Check that externally visible `val`s or `def`s have empty capture sets. If not,
* suggest an explicit type. This is so that separate compilation (where external
* symbols have empty capture sets) gives the same results as joint compilation.
* - Check that arguments of TypeApplys and AppliedTypes conform to their bounds.
*/
def postCheck(unit: tpd.Tree)(using Context): Unit =
unit.foreachSubTree {
Expand All @@ -885,15 +899,23 @@ class CheckCaptures extends Recheck, SymTransformer:
val isLocal =
sym.owner.ownersIterator.exists(_.isTerm)
|| sym.accessBoundary(defn.RootClass).isContainedIn(sym.topLevelClass)

// The following classes of definitions need explicit capture types ...
if !isLocal // ... since external capture types are not inferred
|| sym.owner.is(Trait) // ... since we do OverridingPairs checking before capture inference
|| sym.allOverriddenSymbols.nonEmpty // ... since we do override checking before capture inference
then
def canUseInferred = // If canUseInferred is false, all capturing types in the type of `sym` need to be given explicitly
sym.is(Private) // private symbols can always have inferred types
|| sym.name.is(DefaultGetterName) // default getters are exempted since otherwise it would be
// too annoying. This is a hole since a defualt getter's result type
// might leak into a type variable.
|| // non-local symbols cannot have inferred types since external capture types are not inferred
isLocal // local symbols still need explicit types if
&& !sym.owner.is(Trait) // they are defined in a trait, since we do OverridingPairs checking before capture inference
def isNotPureThis(ref: CaptureRef) = ref match {
case ref: ThisType => !ref.cls.isPureClass
case _ => true
}
if !canUseInferred then
val inferred = t.tpt.knownType
def checkPure(tp: Type) = tp match
case CapturingType(_, refs) if !refs.elems.isEmpty =>
case CapturingType(_, refs)
if !refs.elems.filter(isNotPureThis).isEmpty =>
val resultStr = if t.isInstanceOf[DefDef] then " result" else ""
report.error(
em"""Non-local $sym cannot have an inferred$resultStr type
Expand All @@ -902,8 +924,27 @@ class CheckCaptures extends Recheck, SymTransformer:
|The type needs to be declared explicitly.""", t.srcPos)
case _ =>
inferred.foreachPart(checkPure, StopAt.Static)
case t @ TypeApply(fun, args) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This case is missing in the documentation. Shall we add the explanation of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where would you add it? It's basically the normal checking that type arguments must fit into type parameter bounds, taking capture setting into account.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking that it would be good to be explained in the docstring of postCheck, since it only talks about the other two cases but not this newly-added one for bounds checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that makes sense.

fun.knownType.widen match
case tl: PolyType =>
val normArgs = args.lazyZip(tl.paramInfos).map { (arg, bounds) =>
arg.withType(arg.knownType.forceBoxStatus(
bounds.hi.isBoxedCapturing | bounds.lo.isBoxedCapturing))
}
checkBounds(normArgs, tl)
case _ =>
case _ =>
}

if !ctx.reporter.errorsReported then
// We dont report errors here if previous errors were reported, because other
// errors often result in bad applied types, but flagging these bad types gives
// often worse error messages than the original errors.
val checkApplied = new TreeTraverser:
def traverse(t: Tree)(using Context) = t match
case tree: InferredTypeTree =>
case tree: New =>
case tree: TypeTree => checkAppliedTypesIn(tree.withKnownType)
case _ => traverseChildren(t)
checkApplied.traverse(unit)
end CaptureChecker
end CheckCaptures
Loading