Skip to content

Commit 1303c84

Browse files
committed
Drop dead code, and improve explanation
1 parent 5c7c5a2 commit 1303c84

File tree

1 file changed

+13
-16
lines changed

1 file changed

+13
-16
lines changed

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

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,26 +1060,23 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
10601060

10611061
def checkArgs(): Boolean =
10621062
if tycon1sym == tycon2sym && tycon1sym.isAliasType then
1063-
// if types are simple forwarding aliases then try the dealiased types
1064-
tp1.superType match
1065-
case tp1a as AppliedType(_, args1a) if args1a.eqElements(args1) =>
1066-
tp2.superType match
1067-
case tp2a as AppliedType(_, args2a) if args2a.eqElements(args2) =>
1068-
return recur(tp1a, tp2a)
1069-
case _ =>
1070-
case _ =>
1071-
// Otherwise either compare arguments or compare dealiased types.
1063+
// Either compare arguments or compare dealiased types.
10721064
// Note: This test should be done with an `either`, but this fails
10731065
// for hk-alias-unification.scala The problem is not GADT boundschecking;
10741066
// that is taken care of by the outer inFrozenGadtIf test. The problem is
10751067
// that hk-alias-unification.scala relies on the right isSubArgs being performed
1076-
// when constraining the result type of a method. But there we are only allowed
1077-
// to use a necessaryEither, since it might be that an implicit conversion will
1078-
// be inserted on the result. So strictly speaking by going to a sufficientEither
1079-
// we overshoot and narrow the constraint unnecessarily. On the other hand, this
1080-
// is probably an extreme corner case. If we exclude implicit conversions, the
1081-
// problem goes away since in that case we would have a normal subtype test here,
1082-
// which demands a sufficientEither anyway.
1068+
// when constraining the result type of a method. But since #8635 we are only allowed
1069+
// to use a necessaryEither, since we might otherwise constrain too much.
1070+
// So strictly speaking by going to a sufficientEither we overshoot and narrow
1071+
// the constraint unnecessarily. But unlike in the situation of #8365 with test cases
1072+
// and-inf.scala and or-inf.scala, we don't cut off solutions in principle by dealiasing.
1073+
// We "just" risk missing an opportunity to do a desired hk-type inference. This
1074+
// is hopefully a less frequent corner case. Where it goes wrong compared to the status
1075+
// before this fix: If isSubArgs succeeds and then the dealised test also succeeds
1076+
// while constraining strictly less than the isSubArgs, then the we used to pick the
1077+
// constraint after isSubArgs, but we now pick the constraint after the dealiased test.
1078+
// There's one test in shapeless/deriving that had to be disabled
1079+
// (modules/deriving/src/test/scala/shapeless3/deriving/deriving.scala, value v7 in the functor test).
10831080
sufficientEither(
10841081
isSubArgs(args1, args2, tp1, tparams),
10851082
recur(tp1.superType, tp2.superType))

0 commit comments

Comments
 (0)