Skip to content

Commit 8a5ca00

Browse files
committed
Address review comments
1 parent d2b01d0 commit 8a5ca00

File tree

2 files changed

+17
-14
lines changed

2 files changed

+17
-14
lines changed

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,24 +1172,28 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
11721172
else None
11731173
}
11741174

1175-
/** Check whether we can compare the given set of atoms with another to determine
1176-
* a subtype test between OrTypes. There is one situation where this is not
1177-
* the case, which has to do with SkolemTypes. TreeChecker sometimes expects two
1178-
* types to be equal that have different skolems. To account for this, we identify
1179-
* two different skolems in all phases `p`, where `p.isTyper` is false.
1180-
* But in that case comparing two sets of atoms that contain skolems
1181-
* for equality would give the wrong result, so we should not use the sets
1182-
* for comparisons.
1175+
/** If both `tp1` and `tp2` have atoms information, compare the atoms
1176+
* in a Some, otherwise None.
11831177
*/
11841178
def compareAtoms(tp1: Type, tp2: Type): Option[Boolean] =
1179+
1180+
/** Check whether we can compare the given set of atoms with another to determine
1181+
* a subtype test between OrTypes. There is one situation where this is not
1182+
* the case, which has to do with SkolemTypes. TreeChecker sometimes expects two
1183+
* types to be equal that have different skolems. To account for this, we identify
1184+
* two different skolems in all phases `p`, where `p.isTyper` is false.
1185+
* But in that case comparing two sets of atoms that contain skolems
1186+
* for equality would give the wrong result, so we should not use the sets
1187+
* for comparisons.
1188+
*/
11851189
def canCompare(ts: Set[Type]) = ctx.phase.isTyper || {
11861190
val hasSkolems = new ExistsAccumulator(_.isInstanceOf[SkolemType]) {
11871191
override def stopAtStatic = true
11881192
}
11891193
!ts.exists(hasSkolems(false, _))
11901194
}
11911195
def verified(result: Boolean): Boolean =
1192-
if Config.checkAtomsComparisons && false then
1196+
if Config.checkAtomsComparisons then
11931197
try
11941198
canCompareAtoms = false
11951199
val regular = recur(tp1, tp2)
@@ -1201,16 +1205,14 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
12011205
finally canCompareAtoms = true
12021206
result
12031207

1204-
def falseUnlessBottom = Some(verified(recur(tp1, NothingType)))
1205-
12061208
tp2.atoms match
12071209
case Atoms.Range(lo2, hi2) if canCompareAtoms && canCompare(hi2) =>
12081210
tp1.atoms match
12091211
case Atoms.Range(lo1, hi1) =>
12101212
if hi1.subsetOf(lo2) then Some(verified(true))
1211-
else if !lo1.subsetOf(hi2) then falseUnlessBottom
1213+
else if !lo1.subsetOf(hi2) then Some(verified(false))
12121214
else None
1213-
case _ => falseUnlessBottom
1215+
case _ => Some(verified(recur(tp1, NothingType)))
12141216
case _ => None
12151217

12161218
/** Subtype test for corresponding arguments in `args1`, `args2` according to

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1183,7 +1183,8 @@ object Types {
11831183
case _ => tp
11841184
tp.underlying.atoms match
11851185
case as @ Atoms.Range(lo, hi) =>
1186-
if hi.size == 1 then as else Atoms.Range(Set.empty, hi)
1186+
if hi.size == 1 then as // if there's just one atom, there's no uncertainty which one it is
1187+
else Atoms.Range(Set.empty, hi)
11871188
case Atoms.Unknown =>
11881189
if tp.isStable then
11891190
val single = Set.empty + normalize(tp)

0 commit comments

Comments
 (0)