Skip to content
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

Add path support for capture checking #21445

Merged
merged 12 commits into from
Oct 30, 2024
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
def makeRetaining(parent: Tree, refs: List[Tree], annotName: TypeName)(using Context): Annotated =
Annotated(parent, New(scalaAnnotationDot(annotName), List(refs)))

def makeCapsOf(id: Ident)(using Context): Tree =
TypeApply(Select(scalaDot(nme.caps), nme.capsOf), id :: Nil)
def makeCapsOf(tp: RefTree)(using Context): Tree =
TypeApply(Select(scalaDot(nme.caps), nme.capsOf), tp :: Nil)

def makeCapsBound()(using Context): Tree =
makeRetaining(
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/cc/CaptureOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ extension (tp: Type)
true
case tp: TermRef =>
((tp.prefix eq NoPrefix)
|| tp.symbol.is(ParamAccessor) && tp.prefix.isThisTypeOf(tp.symbol.owner)
|| tp.symbol.isField && !tp.symbol.isStatic && tp.prefix.isTrackableRef
|| tp.isRootCapability
) && !tp.symbol.isOneOf(UnstableValueFlags)
case tp: TypeRef =>
Expand Down
55 changes: 40 additions & 15 deletions compiler/src/dotty/tools/dotc/cc/CaptureRef.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,19 @@ trait CaptureRef extends TypeProxy, ValueType:
case tp: TermParamRef => tp.underlying.derivesFrom(defn.Caps_Exists)
case _ => false

/** Normalize reference so that it can be compared with `eq` for equality */
final def normalizedRef(using Context): CaptureRef = this match
case tp @ AnnotatedType(parent: CaptureRef, annot) if tp.isTrackableRef =>
tp.derivedAnnotatedType(parent.normalizedRef, annot)
case tp: TermRef if tp.isTrackableRef =>
tp.symbol.termRef
case _ => this
// With the support of pathes, we don't need to normalize the `TermRef`s anymore.
// /** Normalize reference so that it can be compared with `eq` for equality */
// final def normalizedRef(using Context): CaptureRef = this match
// case tp @ AnnotatedType(parent: CaptureRef, annot) if tp.isTrackableRef =>
// tp.derivedAnnotatedType(parent.normalizedRef, annot)
// case tp: TermRef if tp.isTrackableRef =>
// tp.symbol.termRef
// case _ => this

/** The capture set consisting of exactly this reference */
final def singletonCaptureSet(using Context): CaptureSet.Const =
if mySingletonCaptureSet == null then
mySingletonCaptureSet = CaptureSet(this.normalizedRef)
mySingletonCaptureSet = CaptureSet(this)
mySingletonCaptureSet.uncheckedNN

/** The capture set of the type underlying this reference */
Expand All @@ -97,27 +98,51 @@ trait CaptureRef extends TypeProxy, ValueType:
* x subsumes y ==> x* subsumes y, x subsumes y?
* x subsumes y ==> x* subsumes y*, x? subsumes y?
* x: x1.type /\ x1 subsumes y ==> x subsumes y
* TODO: Document path cases
*/
final def subsumes(y: CaptureRef)(using Context): Boolean =
Copy link
Contributor

@odersky odersky Sep 24, 2024

Choose a reason for hiding this comment

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

We need to get better reassurance that this does the right thing. It looks too complicated to be able to argue it's obviously correct. What would help:

  • A formal set of rules for subcapturing.
  • A rendition of these rules as a doc comment for this method. It's a shame that the old doc comment was not updated in any way.
  • More extensive tests that try to test all combinations. I saw we have neg tests but no new pos tests.


def subsumingRefs(x: Type, y: Type): Boolean = x match
case x: CaptureRef => y match
case y: CaptureRef => x.subsumes(y)
case _ => false
case _ => false

def viaInfo(info: Type)(test: Type => Boolean): Boolean = info.match
case info: SingletonCaptureRef => test(info)
case info: AndType => viaInfo(info.tp1)(test) || viaInfo(info.tp2)(test)
case info: OrType => viaInfo(info.tp1)(test) && viaInfo(info.tp2)(test)
case _ => false

(this eq y)
|| this.isRootCapability
|| y.match
case y: TermRef =>
(y.prefix eq this)
|| y.info.match
case y1: SingletonCaptureRef => this.subsumes(y1)
y.prefix.match
case ypre: CaptureRef =>
this.subsumes(ypre)
|| this.match
case x @ TermRef(xpre: CaptureRef, _) if x.symbol == y.symbol =>
// To show `{x.f} <:< {y.f}`, it is important to prove `x` and `y`
// are equvalent, which means `x =:= y` in terms of subtyping,
// not just `{x} =:= {y}` in terms of subcapturing.
// It is possible to construct two singleton types `x` and `y`,
// which subsume each other, but are not equal references.
// See `tests/neg-custom-args/captures/path-prefix.scala` for example.
withMode(Mode.IgnoreCaptures) {TypeComparer.isSameRef(xpre, ypre)}
noti0na1 marked this conversation as resolved.
Show resolved Hide resolved
case _ =>
false
case _ => false
|| viaInfo(y.info)(subsumingRefs(this, _))
case MaybeCapability(y1) => this.stripMaybe.subsumes(y1)
case _ => false
|| this.match
case ReachCapability(x1) => x1.subsumes(y.stripReach)
case x: TermRef =>
x.info match
case x1: SingletonCaptureRef => x1.subsumes(y)
case _ => false
case x: TermRef => viaInfo(x.info)(subsumingRefs(_, y))
case x: TermParamRef => subsumesExistentially(x, y)
case x: TypeRef => assumedContainsOf(x).contains(y)
case _ => false
end subsumes

def assumedContainsOf(x: TypeRef)(using Context): SimpleIdentitySet[CaptureRef] =
CaptureSet.assumedContains.getOrElse(x, SimpleIdentitySet.empty)
Expand Down
8 changes: 6 additions & 2 deletions compiler/src/dotty/tools/dotc/cc/CaptureSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ object CaptureSet:

def apply(elems: CaptureRef*)(using Context): CaptureSet.Const =
if elems.isEmpty then empty
else Const(SimpleIdentitySet(elems.map(_.normalizedRef.ensuring(_.isTrackableRef))*))
else Const(SimpleIdentitySet(elems.map(_.ensuring(_.isTrackableRef))*))

def apply(elems: Refs)(using Context): CaptureSet.Const =
if elems.isEmpty then empty else Const(elems)
Expand Down Expand Up @@ -508,7 +508,11 @@ object CaptureSet:
!noUniversal
else elem match
case elem: TermRef if level.isDefined =>
elem.symbol.ccLevel <= level
elem.prefix match
case prefix: CaptureRef =>
levelOK(prefix)
case _ =>
elem.symbol.ccLevel <= level
case elem: ThisType if level.isDefined =>
elem.cls.ccLevel.nextInner <= level
case ReachCapability(elem1) =>
Expand Down
Loading
Loading