Skip to content

Commit ab1c90a

Browse files
committed
Simplify merging in lub/glb, avoid unnecessary constraints
In `mergeIfSuper`, to simplify `tp1 | tp2`, we first check if `tp2` can be made a subtype of `tp1`. If so we could just return `tp1` but this isn't what we did before this commit: at that point we checked if `tp1` could be made a subtype of `tp2` and in that case prefered returning `tp2` to `tp1`. I haven't been able to find a reason for this (the comment says "keep existing type if possible" which I don't understand). On the other hand, I've found cases where this logic broke type inference because the second subtype check inferred extraneous constraints (see added testcase). So this commit simply removes this logic, it does the same for `mergeIfSub` which contains similar logic to simplify `tp1 & tp2`, though this one is less problematic since it always runs with frozen constraints.
1 parent 266a1e4 commit ab1c90a

File tree

3 files changed

+14
-6
lines changed

3 files changed

+14
-6
lines changed

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1983,8 +1983,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
19831983
/** Merge `t1` into `tp2` if t1 is a subtype of some &-summand of tp2.
19841984
*/
19851985
private def mergeIfSub(tp1: Type, tp2: Type): Type =
1986-
if (isSubTypeWhenFrozen(tp1, tp2))
1987-
if (isSubTypeWhenFrozen(tp2, tp1)) tp2 else tp1 // keep existing type if possible
1986+
if (isSubTypeWhenFrozen(tp1, tp2)) tp1
19881987
else tp2 match {
19891988
case tp2 @ AndType(tp21, tp22) =>
19901989
val lower1 = mergeIfSub(tp1, tp21)
@@ -2004,8 +2003,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling[AbsentContext] w
20042003
* @param canConstrain If true, new constraints might be added to make the merge possible.
20052004
*/
20062005
private def mergeIfSuper(tp1: Type, tp2: Type, canConstrain: Boolean): Type =
2007-
if (isSubType(tp2, tp1, whenFrozen = !canConstrain))
2008-
if (isSubType(tp1, tp2, whenFrozen = !canConstrain)) tp2 else tp1 // keep existing type if possible
2006+
if (isSubType(tp2, tp1, whenFrozen = !canConstrain)) tp1
20092007
else tp2 match {
20102008
case tp2 @ OrType(tp21, tp22) =>
20112009
val higher1 = mergeIfSuper(tp1, tp21, canConstrain)

tests/neg/i4564.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,4 @@ class BaseCNSP[T] {
4242
}
4343

4444
object ClashNoSigPoly extends BaseCNSP[Int]
45-
// TODO: improve error message
46-
case class ClashNoSigPoly private(x: Int) // error: found: ClashNoSigPoly required: Nothing
45+
case class ClashNoSigPoly private(x: Int)

tests/pos/merge-constraint.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
class Hi
2+
class Lo extends Hi
3+
4+
object Test {
5+
def foo[T, U <: T](t: T, f: T => U): U = ???
6+
7+
def test(hi: Hi, lo: Lo): Unit = {
8+
val ret = foo(hi, x => lo) // This used to infer U := Hi
9+
val y: Lo = ret
10+
}
11+
}

0 commit comments

Comments
 (0)