Skip to content

Commit d72e5ea

Browse files
oderskyodersky
committed
Fix override checking of alias vs abstract types
# Conflicts: # compiler/src/dotty/tools/dotc/cc/Setup.scala
1 parent c044420 commit d72e5ea

File tree

7 files changed

+125
-36
lines changed

7 files changed

+125
-36
lines changed

‎compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ class CheckCaptures extends Recheck, SymTransformer:
13411341
else if !owner.exists then false
13421342
else isPure(owner.info) && isPureContext(owner.owner, limit)
13431343

1344-
// Augment expeced capture set `erefs` by all references in actual capture
1344+
// Augment expected capture set `erefs` by all references in actual capture
13451345
// set `arefs` that are outside some `C.this.type` reference in `erefs` for an enclosing
13461346
// class `C`. If an added reference is not a ThisType itself, add it to the capture set
13471347
// (i.e. use set) of the `C`. This makes sure that any outer reference implicitly subsumed
@@ -1574,7 +1574,7 @@ class CheckCaptures extends Recheck, SymTransformer:
15741574
/** Check subtype with box adaptation.
15751575
* This function is passed to RefChecks to check the compatibility of overriding pairs.
15761576
* @param sym symbol of the field definition that is being checked
1577-
*/
1577+
15781578
override def checkSubType(actual: Type, expected: Type)(using Context): Boolean =
15791579
val expected1 = alignDependentFunction(addOuterRefs(expected, actual, tree.srcPos), actual.stripCapturing)
15801580
val actual1 =
@@ -1593,11 +1593,44 @@ class CheckCaptures extends Recheck, SymTransformer:
15931593
case _ => adapted
15941594
finally curEnv = saved
15951595
actual1 frozen_<:< expected1
1596+
*/
15961597

15971598
/** Omit the check if one of {overriding,overridden} was nnot capture checked */
15981599
override def needsCheck(overriding: Symbol, overridden: Symbol)(using Context): Boolean =
15991600
!setup.isPreCC(overriding) && !setup.isPreCC(overridden)
16001601

1602+
/** Perform box adaptation for override checking */
1603+
override def adapt(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] =
1604+
if member.isType then
1605+
memberTp match
1606+
case TypeAlias(_) =>
1607+
otherTp match
1608+
case otherTp: RealTypeBounds
1609+
if otherTp.hi.isBoxedCapturing || otherTp.lo.isBoxedCapturing =>
1610+
Some((memberTp, otherTp.unboxed))
1611+
case _ => None
1612+
case _ => None
1613+
else
1614+
val expected1 = alignDependentFunction(addOuterRefs(otherTp, memberTp, tree.srcPos), memberTp.stripCapturing)
1615+
val actual1 =
1616+
val saved = curEnv
1617+
try
1618+
curEnv = Env(clazz, EnvKind.NestedInOwner, capturedVars(clazz), outer0 = curEnv)
1619+
val adapted =
1620+
adaptBoxed(memberTp, expected1, tree, covariant = true, alwaysConst = true)
1621+
memberTp match
1622+
case _: MethodType =>
1623+
// We remove the capture set resulted from box adaptation for method types,
1624+
// since class methods are always treated as pure, and their captured variables
1625+
// are charged to the capture set of the class (which is already done during
1626+
// box adaptation).
1627+
adapted.stripCapturing
1628+
case _ => adapted
1629+
finally curEnv = saved
1630+
if (actual1 eq memberTp) && (expected1 eq otherTp) then None
1631+
else Some((actual1, expected1))
1632+
end adapt
1633+
16011634
override def checkInheritedTraitParameters: Boolean = false
16021635

16031636
/** Check that overrides don't change the @use or @consume status of their parameters */
@@ -1872,7 +1905,9 @@ class CheckCaptures extends Recheck, SymTransformer:
18721905
def traverse(t: Tree)(using Context) = t match
18731906
case tree: InferredTypeTree =>
18741907
case tree: New =>
1875-
case tree: TypeTree => checkAppliedTypesIn(tree.withType(tree.nuType))
1908+
case tree: TypeTree =>
1909+
CCState.withCapAsRoot:
1910+
checkAppliedTypesIn(tree.withType(tree.nuType))
18761911
case _ => traverseChildren(t)
18771912
checkApplied.traverse(unit)
18781913
end postCheck

‎compiler/src/dotty/tools/dotc/cc/Setup.scala

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,20 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
336336
def fail(msg: Message) =
337337
if !tptToCheck.isEmpty then report.error(msg, tptToCheck.srcPos)
338338

339+
/** If C derives from Capability and we have a C^cs in source, we leave it as is
340+
* instead of expanding it to C^{cap.rd}^cs. We do this by stripping capability-generated
341+
* universal capture sets from the parent of a CapturingType.
342+
*/
343+
def stripImpliedCaptureSet(tp: Type): Type = tp match
344+
case tp @ CapturingType(parent, refs)
345+
if (refs eq CaptureSet.universalImpliedByCapability) && !tp.isBoxedCapturing =>
346+
parent
347+
case tp: AliasingBounds =>
348+
tp.derivedAlias(stripImpliedCaptureSet(tp.alias))
349+
case tp: RealTypeBounds =>
350+
tp.derivedTypeBounds(stripImpliedCaptureSet(tp.lo), stripImpliedCaptureSet(tp.hi))
351+
case _ => tp
352+
339353
object toCapturing extends DeepTypeMap, SetupTypeMap:
340354
override def toString = "transformExplicitType"
341355

@@ -367,16 +381,6 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
367381
CapturingType(fntpe, cs, boxed = false)
368382
else fntpe
369383

370-
/** If C derives from Capability and we have a C^cs in source, we leave it as is
371-
* instead of expanding it to C^{cap.rd}^cs. We do this by stripping capability-generated
372-
* universal capture sets from the parent of a CapturingType.
373-
*/
374-
def stripImpliedCaptureSet(tp: Type): Type = tp match
375-
case tp @ CapturingType(parent, refs)
376-
if (refs eq CaptureSet.universalImpliedByCapability) && !tp.isBoxedCapturing =>
377-
parent
378-
case _ => tp
379-
380384
/** Check that types extending SharedCapability don't have a `cap` in their capture set.
381385
* TODO This is not enough.
382386
* We need to also track that we cannot get exclusive capabilities in paths
@@ -456,8 +460,11 @@ class Setup extends PreRecheck, SymTransformer, SetupAPI:
456460
toCapturing.keepFunAliases = false
457461
transform(tp1)
458462
else tp1
459-
if freshen then root.capToFresh(tp2).tap(addOwnerAsHidden(_, sym))
460-
else tp2
463+
val tp3 =
464+
if sym.isType then stripImpliedCaptureSet(tp2)
465+
else tp2
466+
if freshen then root.capToFresh(tp3).tap(addOwnerAsHidden(_, sym))
467+
else tp3
461468
end transformExplicitType
462469

463470
/** Substitute parameter symbols in `from` to paramRefs in corresponding

‎compiler/src/dotty/tools/dotc/cc/root.scala

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ object root:
162162
case _ if root.isCap => Some(Kind.Global)
163163
case _ => None
164164

165-
/** Map each occurrence of cap to a different Sep.Cap instance */
165+
/** Map each occurrence of cap to a different Fresh instance
166+
* Exception: CapSet^ stays as it is.
167+
*/
166168
class CapToFresh(owner: Symbol)(using Context) extends BiTypeMap, FollowAliasesMap:
167169
thisMap =>
168170

@@ -171,6 +173,8 @@ object root:
171173
else t match
172174
case t: CaptureRef if t.isCap =>
173175
Fresh.withOwner(owner)
176+
case t @ CapturingType(parent: TypeRef, _) if parent.symbol == defn.Caps_CapSet =>
177+
t
174178
case t @ CapturingType(_, _) =>
175179
mapOver(t)
176180
case t @ AnnotatedType(parent, ann) =>

‎compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
426426
if (tp1.prefix.isStable) return tryLiftedToThis1
427427
case _ =>
428428
if isCaptureVarComparison then
429-
return subCaptures(tp1.captureSet, tp2.captureSet).isOK
429+
return CCState.withCapAsRoot:
430+
subCaptures(tp1.captureSet, tp2.captureSet).isOK
430431
if (tp1 eq NothingType) || isBottom(tp1) then
431432
return true
432433
}
@@ -575,7 +576,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
575576
&& (isBottom(tp1) || GADTusage(tp2.symbol))
576577

577578
if isCaptureVarComparison then
578-
return subCaptures(tp1.captureSet, tp2.captureSet).isOK
579+
return CCState.withCapAsRoot:
580+
subCaptures(tp1.captureSet, tp2.captureSet).isOK
579581

580582
isSubApproxHi(tp1, info2.lo) && (trustBounds || isSubApproxHi(tp1, info2.hi))
581583
|| compareGADT

‎compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import NameKinds.DefaultGetterName
88
import NullOpsDecorator.*
99
import collection.immutable.BitSet
1010
import scala.annotation.tailrec
11-
import cc.isCaptureChecking
11+
import cc.{isCaptureChecking, CCState}
1212

1313
import scala.compiletime.uninitialized
1414

@@ -215,14 +215,15 @@ object OverridingPairs:
215215
if member.isType then // intersection of bounds to refined types must be nonempty
216216
memberTp.bounds.hi.hasSameKindAs(otherTp.bounds.hi)
217217
&& (
218-
(memberTp frozen_<:< otherTp)
219-
|| !member.owner.derivesFrom(other.owner)
220-
&& {
221-
// if member and other come from independent classes or traits, their
222-
// bounds must have non-empty-intersection
223-
val jointBounds = (memberTp.bounds & otherTp.bounds).bounds
224-
jointBounds.lo frozen_<:< jointBounds.hi
225-
}
218+
CCState.withCapAsRoot: // If upper bound is CapSet^ any capture set of lower bound is OK
219+
(memberTp frozen_<:< otherTp)
220+
|| !member.owner.derivesFrom(other.owner)
221+
&& {
222+
// if member and other come from independent classes or traits, their
223+
// bounds must have non-empty-intersection
224+
val jointBounds = (memberTp.bounds & otherTp.bounds).bounds
225+
jointBounds.lo frozen_<:< jointBounds.hi
226+
}
226227
)
227228
else
228229
member.name.is(DefaultGetterName) // default getters are not checked for compatibility

‎compiler/src/dotty/tools/dotc/typer/RefChecks.scala

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,13 @@ object RefChecks {
253253
*/
254254
def needsCheck(overriding: Symbol, overridden: Symbol)(using Context): Boolean = true
255255

256+
/** Adapt member type and other type so that they can be compared with `frozen_<:<`.
257+
* @return optionally, if adaptation is necessary, the pair of adapted types (memberTp', otherTp')
258+
* Note: we return an Option result to avoid a tuple allocation in the normal case
259+
* where no adaptation is necessary.
260+
*/
261+
def adapt(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] = None
262+
256263
protected def additionalChecks(overriding: Symbol, overridden: Symbol)(using Context): Unit = ()
257264

258265
private val subtypeChecker: (Type, Type) => Context ?=> Boolean = this.checkSubType
@@ -429,18 +436,26 @@ object RefChecks {
429436
* of class `clazz` are met.
430437
*/
431438
def checkOverride(checkSubType: (Type, Type) => Context ?=> Boolean, member: Symbol, other: Symbol): Unit =
432-
def memberTp(self: Type) =
439+
def memberType(self: Type) =
433440
if (member.isClass) TypeAlias(member.typeRef.etaExpand)
434441
else self.memberInfo(member)
435-
def otherTp(self: Type) =
436-
self.memberInfo(other)
442+
def otherType(self: Type) =
443+
self.memberInfo(other)
444+
445+
var memberTp = memberType(self)
446+
var otherTp = otherType(self)
447+
checker.adapt(member, memberTp, otherTp) match
448+
case Some((mtp, otp)) =>
449+
memberTp = mtp
450+
otherTp = otp
451+
case None =>
437452

438453
refcheck.println(i"check override ${infoString(member)} overriding ${infoString(other)}")
439454

440-
def noErrorType = !memberTp(self).isErroneous && !otherTp(self).isErroneous
455+
def noErrorType = !memberTp.isErroneous && !otherTp.isErroneous
441456

442457
def overrideErrorMsg(core: Context ?=> String, compareTypes: Boolean = false): Message =
443-
val (mtp, otp) = if compareTypes then (memberTp(self), otherTp(self)) else (NoType, NoType)
458+
val (mtp, otp) = if compareTypes then (memberTp, otherTp) else (NoType, NoType)
444459
OverrideError(core, self, member, other, mtp, otp)
445460

446461
def compatTypes(memberTp: Type, otherTp: Type): Boolean =
@@ -469,7 +484,7 @@ object RefChecks {
469484
// with box adaptation, we simply ignore capture annotations here.
470485
// This should be safe since the compatibility under box adaptation is already
471486
// checked.
472-
memberTp(self).matches(otherTp(self))
487+
memberTp.matches(otherTp)
473488
}
474489

475490
def emitOverrideError(fullmsg: Message) =
@@ -624,20 +639,29 @@ object RefChecks {
624639
overrideError("is not inline, cannot implement an inline method")
625640
else if (other.isScala2Macro && !member.isScala2Macro) // (1.11)
626641
overrideError("cannot be used here - only Scala-2 macros can override Scala-2 macros")
627-
else if !compatTypes(memberTp(self), otherTp(self))
628-
&& !compatTypes(memberTp(upwardsSelf), otherTp(upwardsSelf))
642+
else if !compatTypes(memberTp, otherTp)
629643
&& !member.is(Tracked)
630644
// Tracked members need to be excluded since they are abstract type members with
631645
// singleton types. Concrete overrides usually have a wider type.
632646
// TODO: Should we exclude all refinements inherited from parents?
647+
&& {
648+
var memberTpUp = memberType(upwardsSelf)
649+
var otherTpUp = otherType(upwardsSelf)
650+
checker.adapt(member, memberTpUp, otherTpUp) match
651+
case Some((mtp, otp)) =>
652+
memberTpUp = mtp
653+
otherTpUp = otp
654+
case _ =>
655+
!compatTypes(memberTpUp, otherTpUp)
656+
}
633657
then
634658
overrideError("has incompatible type", compareTypes = true)
635659
else if (member.targetName != other.targetName)
636660
if (other.targetName != other.name)
637661
overrideError(i"needs to be declared with @targetName(${"\""}${other.targetName}${"\""}) so that external names match")
638662
else
639663
overrideError("cannot have a @targetName annotation since external names would be different")
640-
else if intoOccurrences(memberTp(self)) != intoOccurrences(otherTp(self)) then
664+
else if intoOccurrences(memberTp) != intoOccurrences(otherTp) then
641665
overrideError("has different occurrences of `into` modifiers", compareTypes = true)
642666
else if other.is(ParamAccessor) && !isInheritedAccessor(member, other)
643667
&& !member.is(Tracked) // see remark on tracked members above

‎tests/pos-custom-args/captures/check-override-typebounds.scala

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import language.experimental.captureChecking
2+
3+
trait A:
4+
type T <: caps.Capability
5+
6+
class B extends A:
7+
type T = C
8+
9+
class C extends caps.Capability
10+
11+
12+
trait A2:
13+
type T[Cap^]
14+
15+
def takesCap[Cap^](t: T[Cap]): Unit
16+

0 commit comments

Comments
 (0)