Don't align aliases in refined types by default

We previously tried to force S1 and S2 be the same type
when encountering a lub like `T1 { A = S1 } & T2 { A = S2 }`.
The comments in this commit explain why this is unsound, so
this rewrite is now made subject to a new config option, which
is off by default.

I verified that the new behavior does not affect the performance of
the junit tests.

2 files changed, 34 insertions, 14 deletions

diff --git a/compiler/src/dotty/tools/dotc/config/Config.scala b/compiler/src/dotty/tools/dotc/config/Config.scala
index 119af9483..900e5669f 100644
--- a/compiler/src/dotty/tools/dotc/config/Config.scala
+++ b/compiler/src/dotty/tools/dotc/config/Config.scala
@@ -75,6 +75,21 @@ object Config {
   /** If this flag is set, take the fast path when comparing same-named type-aliases and types */
   final val fastPathForRefinedSubtype = true
 
+  /** If this flag is set, and we compute `T1 { X = S1 }` & `T2 { X = S2 }`,
+   *  try to align the refinements by computing `S1 =:= S2` (which might instantiate type parameters).
+   *  This rule is contentious because it cuts the constraint set. Also, it is
+   *  currently unsound because `&` gets called in computations on a constraint
+   *  itself. If the `=:=` test generates a new constraint, that constraint is then
+   *  out of sync with with the constraint on which the computation is performed.
+   *  The constraint resulting from `=:=` ends up to be thrown away whereas
+   *  its result is used, which is unsound. So if we want to turn this flag on
+   *  permanently instead of just for debugging, we have to refactor occurrences
+   *  of `&` in `OrderedConstraint` so that they take the `=:=` result into account.
+   *
+   *  For more info, see the comment in `TypeComparer#distributeAnd`.
+   */
+  final val alignArgsInAnd = false
+
   /** If this flag is set, higher-kinded applications are checked for validity
    */
   final val checkHKApplications = false
diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
index fca111702..744112280 100644
--- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
+++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
@@ -1312,23 +1312,28 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
     case tp1: RefinedType =>
       tp2 match {
         case tp2: RefinedType if tp1.refinedName == tp2.refinedName =>
-          // Given two refinements `T1 { X = S1 }` and `T2 { X = S2 }`, if `S1 =:= S2`
-          // (possibly by instantiating type parameters), rewrite to `T1 & T2 { X = S1 }`.
-          // Otherwise rewrite to `T1 & T2 { X B }` where `B` is the conjunction of
-          // the bounds of `X` in `T1` and `T2`.
-          // The first rule above is contentious because it cuts the constraint set.
-          // But without it we would replace the two aliases by
-          // `T { X >: S1 | S2 <: S1 & S2 }`, which looks weird and is probably
-          // not what's intended.
+          // Given two refinements `T1 { X = S1 }` and `T2 { X = S2 }` rwrite to
+          // `T1 & T2 { X B }` where `B` is the conjunction of the bounds of `X` in `T1` and `T2`.
+          //
+          // However, if `Config.alignArgsInAnd` is set, and both aliases `X = Si` are
+          // nonvariant, and `S1 =:= S2` (possibly by instantiating type parameters),
+          // rewrite instead to `T1 & T2 { X = S1 }`. This rule is contentious because
+          // it cuts the constraint set. On the other hand, without it we would replace
+          // the two aliases by `T { X >: S1 | S2 <: S1 & S2 }`, which looks weird
+          // and is probably not what's intended.
           val rinfo1 = tp1.refinedInfo
           val rinfo2 = tp2.refinedInfo
           val parent = tp1.parent & tp2.parent
-          val rinfo =
-            if (rinfo1.isAlias && rinfo2.isAlias && isSameType(rinfo1, rinfo2))
-              rinfo1
-            else
-              rinfo1 & rinfo2
-          tp1.derivedRefinedType(parent, tp1.refinedName, rinfo)
+
+          def isNonvariantAlias(tp: Type) = tp match {
+            case tp: TypeAlias => tp.variance == 0
+            case _ => false
+          }
+          if (Config.alignArgsInAnd &&
+              isNonvariantAlias(rinfo1) && isNonvariantAlias(rinfo2))
+            isSameType(rinfo1, rinfo2)
+
+          tp1.derivedRefinedType(parent, tp1.refinedName, rinfo1 & rinfo2)
         case _ =>
           NoType
       }