Make alignArgsInAnd safe and turn it on by default

Turned out hmaps.scala requires the arg alignment to compile.
So we have our first counterexample that we cannot drop this
hack. Now it is made safe in the sense that no constraints get
lost anymore.

Author: odersky

compiler/src/dotty/tools/dotc/config/Config.scala

@@ -75,20 +75,14 @@ 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.
+  /** If this flag is set, and we compute `T1 { X = S1 }` & `T2 { X = S2 }` as a new
+   *  upper bound of a constrained parameter, try to align the refinements by computing
+   *  `S1 =:= S2` (which might instantiate type parameters).
+   *  This rule is contentious because it cuts the constraint set.
    *
    *  For more info, see the comment in `TypeComparer#distributeAnd`.
    */
-  final val alignArgsInAnd = false
+  final val alignArgsInAnd = true
 
   /** If this flag is set, higher-kinded applications are checked for validity
    */

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

@@ -111,12 +111,6 @@ abstract class Constraint extends Showable {
    */
   def replace(param: PolyParam, tp: Type)(implicit ctx: Context): This
 
-  /** Narrow one of the bounds of type parameter `param`
-   *  If `isUpper` is true, ensure that `param <: `bound`, otherwise ensure
-   *  that `param >: bound`.
-   */
-  def narrowBound(param: PolyParam, bound: Type, isUpper: Boolean)(implicit ctx: Context): This
-
   /** Is entry associated with `pt` removable? This is the case if
    *  all type parameters of the entry are associated with type variables
    *  which have their `inst` fields set.

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

@@ -44,6 +44,13 @@ trait ConstraintHandling {
     try op finally alwaysFluid = saved
   }
 
+  /** If set, align arguments `S1`, `S2`when taking the glb
+   *  `T1 { X = S1 } & T2 { X = S2 }` of a constraint upper bound for some type parameter.
+   *  Aligning means computing `S1 =:= S2` which may change the current constraint.
+   *  See note in TypeComparer#distributeAnd.
+   */
+  protected var homogenizeArgs = false
+
   /** We are currently comparing polytypes. Used as a flag for
    *  optimization: when `false`, no need to do an expensive `pruneLambdaParams`
    */
@@ -64,14 +71,29 @@ trait ConstraintHandling {
       }
       if (Config.checkConstraintsSeparated)
         assert(!occursIn(bound), s"$param occurs in $bound")
-      val c1 = constraint.narrowBound(param, bound, isUpper)
+      val newBound = narrowedBound(param, bound, isUpper)
+      val c1 = constraint.updateEntry(param, newBound)
       (c1 eq constraint) || {
         constraint = c1
         val TypeBounds(lo, hi) = constraint.entry(param)
         isSubType(lo, hi)
       }
     }
 
+  /** Narrow one of the bounds of type parameter `param`
+   *  If `isUpper` is true, ensure that `param <: `bound`, otherwise ensure
+   *  that `param >: bound`.
+   */
+  def narrowedBound(param: PolyParam, bound: Type, isUpper: Boolean)(implicit ctx: Context): TypeBounds = {
+    val oldBounds @ TypeBounds(lo, hi) = constraint.nonParamBounds(param)
+    val saved = homogenizeArgs
+    homogenizeArgs = Config.alignArgsInAnd
+    try
+      if (isUpper) oldBounds.derivedTypeBounds(lo, hi & bound)
+      else oldBounds.derivedTypeBounds(lo | bound, hi)
+    finally homogenizeArgs = saved
+  }
+
   protected def addUpperBound(param: PolyParam, bound: Type): Boolean = {
     def description = i"constraint $param <: $bound to\n$constraint"
     if (bound.isRef(defn.NothingClass) && ctx.typerState.isGlobalCommittable) {

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

@@ -354,14 +354,6 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
     updateEntry(p1, p1Bounds).replace(p2, p1)
   }
 
-  def narrowBound(param: PolyParam, bound: Type, isUpper: Boolean)(implicit ctx: Context): This = {
-    val oldBounds @ TypeBounds(lo, hi) = nonParamBounds(param)
-    val newBounds =
-      if (isUpper) oldBounds.derivedTypeBounds(lo, hi & bound)
-      else oldBounds.derivedTypeBounds(lo | bound, hi)
-    updateEntry(param, newBounds)
-  }
-
 // ---------- Removals ------------------------------------------------------------
 
   /** A new constraint which is derived from this constraint by removing

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

@@ -1315,7 +1315,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
           // 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
+          // However, if `homogenizeArgs` 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
@@ -1329,7 +1329,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
             case tp: TypeAlias => tp.variance == 0
             case _ => false
           }
-          if (Config.alignArgsInAnd &&
+          if (homogenizeArgs &&
               isNonvariantAlias(rinfo1) && isNonvariantAlias(rinfo2))
             isSameType(rinfo1, rinfo2)