Skip to content

Commit 19dcfa2

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

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

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

16021579
/** Perform box adaptation for override checking */
1603-
override def adapt(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] =
1580+
override def adaptOverridePair(member: Symbol, memberTp: Type, otherTp: Type)(using Context): Option[(Type, Type)] =
16041581
if member.isType then
16051582
memberTp match
16061583
case TypeAlias(_) =>
@@ -1610,26 +1587,36 @@ class CheckCaptures extends Recheck, SymTransformer:
16101587
Some((memberTp, otherTp.unboxed))
16111588
case _ => None
16121589
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
1590+
else memberTp match
1591+
case memberTp @ ExprType(memberRes) =>
1592+
adaptOverridePair(member, memberRes, otherTp) match
1593+
case Some((mres, otp)) => Some((memberTp.derivedExprType(mres), otp))
1594+
case None => None
1595+
case _ => otherTp match
1596+
case otherTp @ ExprType(otherRes) =>
1597+
adaptOverridePair(member, memberTp, otherRes) match
1598+
case Some((mtp, ores)) => Some((mtp, otherTp.derivedExprType(ores)))
1599+
case None => None
1600+
case _ =>
1601+
val expected1 = alignDependentFunction(addOuterRefs(otherTp, memberTp, tree.srcPos), memberTp.stripCapturing)
1602+
val actual1 =
1603+
val saved = curEnv
1604+
try
1605+
curEnv = Env(clazz, EnvKind.NestedInOwner, capturedVars(clazz), outer0 = curEnv)
1606+
val adapted =
1607+
adaptBoxed(memberTp, expected1, tree, covariant = true, alwaysConst = true)
1608+
memberTp match
1609+
case _: MethodType =>
1610+
// We remove the capture set resulted from box adaptation for method types,
1611+
// since class methods are always treated as pure, and their captured variables
1612+
// are charged to the capture set of the class (which is already done during
1613+
// box adaptation).
1614+
adapted.stripCapturing
1615+
case _ => adapted
1616+
finally curEnv = saved
1617+
if (actual1 eq memberTp) && (expected1 eq otherTp) then None
1618+
else Some((actual1, expected1))
1619+
end adaptOverridePair
16331620

16341621
override def checkInheritedTraitParameters: Boolean = false
16351622

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)