Skip to content

Commit be6968e

Browse files
committed
Refactor: Drop isSubType parameter for override checking
Simplifies previous too convoluted logic.
1 parent a94efa4 commit be6968e

File tree

4 files changed

+44
-67
lines changed

4 files changed

+44
-67
lines changed

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

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,36 +1574,13 @@ class CheckCaptures extends Recheck, SymTransformer:
15741574
*/
15751575
def checkOverrides = new TreeTraverser:
15761576
class OverridingPairsCheckerCC(clazz: ClassSymbol, self: Type, tree: Tree)(using Context) extends OverridingPairsChecker(clazz, self):
1577-
/** Check subtype with box adaptation.
1578-
* This function is passed to RefChecks to check the compatibility of overriding pairs.
1579-
* @param sym symbol of the field definition that is being checked
1580-
1581-
override def checkSubType(actual: Type, expected: Type)(using Context): Boolean =
1582-
val expected1 = alignDependentFunction(addOuterRefs(expected, actual, tree.srcPos), actual.stripCapturing)
1583-
val actual1 =
1584-
val saved = curEnv
1585-
try
1586-
curEnv = Env(clazz, EnvKind.NestedInOwner, capturedVars(clazz), outer0 = curEnv)
1587-
val adapted =
1588-
adaptBoxed(actual, expected1, tree, covariant = true, alwaysConst = true)
1589-
actual match
1590-
case _: MethodType =>
1591-
// We remove the capture set resulted from box adaptation for method types,
1592-
// since class methods are always treated as pure, and their captured variables
1593-
// are charged to the capture set of the class (which is already done during
1594-
// box adaptation).
1595-
adapted.stripCapturing
1596-
case _ => adapted
1597-
finally curEnv = saved
1598-
actual1 frozen_<:< expected1
1599-
*/
16001577

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

16051582
/** Perform box adaptation for override checking */
1606-
override def adapt(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] =
1583+
override def adaptOverridePair(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] =
16071584
if member.isType then
16081585
memberTp match
16091586
case TypeAlias(_) =>
@@ -1613,26 +1590,36 @@ class CheckCaptures extends Recheck, SymTransformer:
16131590
Some((memberTp, otherTp.unboxed))
16141591
case _ => None
16151592
case _ => None
1616-
else
1617-
val expected1 = alignDependentFunction(addOuterRefs(otherTp, memberTp, tree.srcPos), memberTp.stripCapturing)
1618-
val actual1 =
1619-
val saved = curEnv
1620-
try
1621-
curEnv = Env(clazz, EnvKind.NestedInOwner, capturedVars(clazz), outer0 = curEnv)
1622-
val adapted =
1623-
adaptBoxed(memberTp, expected1, tree, covariant = true, alwaysConst = true)
1624-
memberTp match
1625-
case _: MethodType =>
1626-
// We remove the capture set resulted from box adaptation for method types,
1627-
// since class methods are always treated as pure, and their captured variables
1628-
// are charged to the capture set of the class (which is already done during
1629-
// box adaptation).
1630-
adapted.stripCapturing
1631-
case _ => adapted
1632-
finally curEnv = saved
1633-
if (actual1 eq memberTp) && (expected1 eq otherTp) then None
1634-
else Some((actual1, expected1))
1635-
end adapt
1593+
else memberTp match
1594+
case memberTp @ ExprType(memberRes) =>
1595+
adaptOverridePair(member, memberRes, otherTp) match
1596+
case Some((mres, otp)) => Some((memberTp.derivedExprType(mres), otp))
1597+
case None => None
1598+
case _ => otherTp match
1599+
case otherTp @ ExprType(otherRes) =>
1600+
adaptOverridePair(member, memberTp, otherRes) match
1601+
case Some((mtp, ores)) => Some((mtp, otherTp.derivedExprType(ores)))
1602+
case None => None
1603+
case _ =>
1604+
val expected1 = alignDependentFunction(addOuterRefs(otherTp, memberTp, tree.srcPos), memberTp.stripCapturing)
1605+
val actual1 =
1606+
val saved = curEnv
1607+
try
1608+
curEnv = Env(clazz, EnvKind.NestedInOwner, capturedVars(clazz), outer0 = curEnv)
1609+
val adapted =
1610+
adaptBoxed(memberTp, expected1, tree, covariant = true, alwaysConst = true)
1611+
memberTp match
1612+
case _: MethodType =>
1613+
// We remove the capture set resulted from box adaptation for method types,
1614+
// since class methods are always treated as pure, and their captured variables
1615+
// are charged to the capture set of the class (which is already done during
1616+
// box adaptation).
1617+
adapted.stripCapturing
1618+
case _ => adapted
1619+
finally curEnv = saved
1620+
if (actual1 eq memberTp) && (expected1 eq otherTp) then None
1621+
else Some((actual1, expected1))
1622+
end adaptOverridePair
16361623

16371624
override def checkInheritedTraitParameters: Boolean = false
16381625

compiler/src/dotty/tools/dotc/core/Types.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,10 +1167,9 @@ object Types extends TypeUtils {
11671167
*
11681168
* @param isSubType a function used for checking subtype relationships.
11691169
*/
1170-
final def overrides(that: Type, matchLoosely: => Boolean, checkClassInfo: Boolean = true,
1171-
isSubType: (Type, Type) => Context ?=> Boolean = (tp1, tp2) => tp1 frozen_<:< tp2)(using Context): Boolean = {
1170+
final def overrides(that: Type, matchLoosely: => Boolean, checkClassInfo: Boolean = true)(using Context): Boolean = {
11721171
!checkClassInfo && this.isInstanceOf[ClassInfo]
1173-
|| isSubType(this.widenExpr, that.widenExpr)
1172+
|| (this.widenExpr frozen_<:< that.widenExpr)
11741173
|| matchLoosely && {
11751174
val this1 = this.widenNullaryMethod
11761175
val that1 = that.widenNullaryMethod

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,8 +210,7 @@ object OverridingPairs:
210210
* @param isSubType A function to be used for checking subtype relationships
211211
* between term fields.
212212
*/
213-
def isOverridingPair(member: Symbol, memberTp: Type, other: Symbol, otherTp: Type, fallBack: => Boolean = false,
214-
isSubType: (Type, Type) => Context ?=> Boolean = (tp1, tp2) => tp1 frozen_<:< tp2)(using Context): Boolean =
213+
def isOverridingPair(member: Symbol, memberTp: Type, other: Symbol, otherTp: Type, fallBack: => Boolean = false)(using Context): Boolean =
215214
if member.isType then // intersection of bounds to refined types must be nonempty
216215
memberTp.bounds.hi.hasSameKindAs(otherTp.bounds.hi)
217216
&& (
@@ -227,6 +226,6 @@ object OverridingPairs:
227226
)
228227
else
229228
member.name.is(DefaultGetterName) // default getters are not checked for compatibility
230-
|| memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack, isSubType = isSubType)
229+
|| memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack)
231230

232231
end OverridingPairs

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

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -242,12 +242,6 @@ object RefChecks {
242242
&& (inLinearizationOrder(sym1, sym2, parent) || parent.is(JavaDefined))
243243
&& !sym2.is(AbsOverride)
244244

245-
/** Checks the subtype relationship tp1 <:< tp2.
246-
* It is passed to the `checkOverride` operation in `checkAll`, to be used for
247-
* compatibility checking.
248-
*/
249-
def checkSubType(tp1: Type, tp2: Type)(using Context): Boolean = tp1 frozen_<:< tp2
250-
251245
/** A hook that allows to omit override checks between `overriding` and `overridden`.
252246
* Overridden in capture checking to handle non-capture checked classes leniently.
253247
*/
@@ -258,16 +252,14 @@ object RefChecks {
258252
* Note: we return an Option result to avoid a tuple allocation in the normal case
259253
* where no adaptation is necessary.
260254
*/
261-
def adapt(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] = None
255+
def adaptOverridePair(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] = None
262256

263257
protected def additionalChecks(overriding: Symbol, overridden: Symbol)(using Context): Unit = ()
264258

265-
private val subtypeChecker: (Type, Type) => Context ?=> Boolean = this.checkSubType
266-
267-
def checkAll(checkOverride: ((Type, Type) => Context ?=> Boolean, Symbol, Symbol) => Unit) =
259+
def checkAll(checkOverride: (Symbol, Symbol) => Unit) =
268260
while hasNext do
269261
if needsCheck(overriding, overridden) then
270-
checkOverride(subtypeChecker, overriding, overridden)
262+
checkOverride(overriding, overridden)
271263
additionalChecks(overriding, overridden)
272264
next()
273265

@@ -282,7 +274,7 @@ object RefChecks {
282274
if dcl.is(Deferred) then
283275
for other <- dcl.allOverriddenSymbols do
284276
if !other.is(Deferred) then
285-
checkOverride(subtypeChecker, dcl, other)
277+
checkOverride(dcl, other)
286278
end checkAll
287279

288280
// Disabled for capture checking since traits can get different parameter refinements
@@ -435,7 +427,7 @@ object RefChecks {
435427
/* Check that all conditions for overriding `other` by `member`
436428
* of class `clazz` are met.
437429
*/
438-
def checkOverride(checkSubType: (Type, Type) => Context ?=> Boolean, member: Symbol, other: Symbol): Unit =
430+
def checkOverride(member: Symbol, other: Symbol): Unit =
439431
def memberType(self: Type) =
440432
if (member.isClass) TypeAlias(member.typeRef.etaExpand)
441433
else self.memberInfo(member)
@@ -444,7 +436,7 @@ object RefChecks {
444436

445437
var memberTp = memberType(self)
446438
var otherTp = otherType(self)
447-
checker.adapt(member, memberTp, otherTp) match
439+
checker.adaptOverridePair(member, memberTp, otherTp) match
448440
case Some((mtp, otp)) =>
449441
memberTp = mtp
450442
otherTp = otp
@@ -463,8 +455,8 @@ object RefChecks {
463455
isOverridingPair(member, memberTp, other, otherTp,
464456
fallBack = warnOnMigration(
465457
overrideErrorMsg("no longer has compatible type"),
466-
(if (member.owner == clazz) member else clazz).srcPos, version = `3.0`),
467-
isSubType = checkSubType)
458+
(if member.owner == clazz then member else clazz).srcPos,
459+
version = `3.0`))
468460
catch case ex: MissingType =>
469461
// can happen when called with upwardsSelf as qualifier of memberTp and otherTp,
470462
// because in that case we might access types that are not members of the qualifier.
@@ -647,7 +639,7 @@ object RefChecks {
647639
&& {
648640
var memberTpUp = memberType(upwardsSelf)
649641
var otherTpUp = otherType(upwardsSelf)
650-
checker.adapt(member, memberTpUp, otherTpUp) match
642+
checker.adaptOverridePair(member, memberTpUp, otherTpUp) match
651643
case Some((mtp, otp)) =>
652644
memberTpUp = mtp
653645
otherTpUp = otp

0 commit comments

Comments
 (0)