Associative, commutative, Java-friendly intersection erasure

Thanks to scala#11603, we can now freely decide how we erase our intersection
types without compromising compatibility with Scala 2. We take advantage
of this by designing a new erasedGlb algorithm which respects
associativity and commutativity and can also erase Java intersections
without any special case.

This commit also improves the way we handle intersections in Java
generic signature to produce more precise types when possible and to
ensure that we never emit a type that would lead to javac emitting
invalid bytecode (which can happen with Scala 2:
scala/bug#12300).

Fixes scala#5139.

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

@@ -2128,7 +2128,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
     else {
       val t2 = distributeAnd(tp2, tp1)
       if (t2.exists) t2
-      else if (isErased) erasedGlb(tp1, tp2, isJava = false)
+      else if (isErased) erasedGlb(tp1, tp2)
       else liftIfHK(tp1, tp2, op, original, _ | _)
         // The ` | ` on variances is needed since variances are associated with bounds
         // not lambdas. Example:

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

@@ -394,32 +394,82 @@ object TypeErasure {
   }
 
   /** The erased greatest lower bound of two erased type picks one of the two argument types.
-   *  It prefers, in this order:
-   *  - arrays over non-arrays
-   *  - subtypes over supertypes, unless isJava is set
-   *  - real classes over traits
+   *
+   *  This operation has the following the properties:
+   *  - Associativity and commutativity, because this method act as the minimum
+   *    of the total order induced by `compareErasedGlb`.
+   *  - Java compatibility: intersections that would be valid in Java code are
+   *    erased like javac would erase them (a Java intersection is composed of
+   *    exactly one class and one or more interfaces and always erases to the
+   *    class).
    */
-  def erasedGlb(tp1: Type, tp2: Type, isJava: Boolean)(using Context): Type = tp1 match {
-    case JavaArrayType(elem1) =>
-      tp2 match {
-        case JavaArrayType(elem2) => JavaArrayType(erasedGlb(elem1, elem2, isJava))
-        case _ => tp1
-      }
-    case _ =>
-      tp2 match {
-        case JavaArrayType(_) => tp2
-        case _ =>
-          val tsym1 = tp1.typeSymbol
-          val tsym2 = tp2.typeSymbol
-          if (!tsym2.exists) tp1
-          else if (!tsym1.exists) tp2
-          else if (!isJava && tsym1.derivesFrom(tsym2)) tp1
-          else if (!isJava && tsym2.derivesFrom(tsym1)) tp2
-          else if (tp1.typeSymbol.isRealClass) tp1
-          else if (tp2.typeSymbol.isRealClass) tp2
-          else tp1
-      }
-  }
+  def erasedGlb(tp1: Type, tp2: Type)(using Context): Type =
+    if compareErasedGlb(tp1, tp2) <= 0 then tp1 else tp2
+
+  /** Overload of `erasedGlb` to compare more than two types at once. */
+  def erasedGlb(tps: List[Type])(using Context): Type =
+    tps.min(using (a,b) => compareErasedGlb(a, b))
+
+  /** A comparison function that induces a total order on erased types,
+   *  where `A <= B` implies that the erasure of `A & B` should be A.
+   *
+   *  This order respects the following properties:
+   *  - ErasedValueTypes <= non-ErasedValueTypes
+   *  - arrays <= non-arrays
+   *  - real classes <= traits
+   *  - subtypes <= supertypes
+   *
+   *  Additionally, lexicographic ordering of the class symbol full name is used
+   *  as a tie-breaker so `A <= B && B <= A` iff `A =:= B`.
+   *
+   *  @see erasedGlb
+   */
+  private def compareErasedGlb(tp1: Type, tp2: Type)(using Context): Int =
+    // this check is purely an optimization.
+    if tp1 eq tp2 then
+      return 0
+
+    val isEVT1 = tp1.isInstanceOf[ErasedValueType]
+    val isEVT2 = tp2.isInstanceOf[ErasedValueType]
+    if isEVT1 && isEVT2 then
+      return compareErasedGlb(tp1.asInstanceOf[ErasedValueType].tycon, tp2.asInstanceOf[ErasedValueType].tycon)
+    else if isEVT1 then
+      return -1
+    else if isEVT2 then
+      return 1
+
+    val isArray1 = tp1.isInstanceOf[JavaArrayType]
+    val isArray2 = tp2.isInstanceOf[JavaArrayType]
+    if isArray1 && isArray2 then
+      return compareErasedGlb(tp1.asInstanceOf[JavaArrayType].elemType, tp2.asInstanceOf[JavaArrayType].elemType)
+    else if isArray1 then
+      return -1
+    else if isArray2 then
+      return 1
+
+    val sym1 = tp1.classSymbol
+    val sym2 = tp2.classSymbol
+    def compareClasses: Int =
+      if sym1.isSubClass(sym2) then
+        -1
+      else if sym2.isSubClass(sym1) then
+        1
+      // Intentionally compare Strings and not Names, since the ordering on
+      // Names depends on implementation details like `NameKind#tag`.
+      else
+        sym1.fullName.toString.compareTo(sym2.fullName.toString)
+
+    val isRealClass1 = sym1.isRealClass
+    val isRealClass2 = sym2.isRealClass
+    if isRealClass1 && isRealClass2 then
+      return compareClasses
+    else if isRealClass1 then
+      return -1
+    else if isRealClass2 then
+      return 1
+
+    compareClasses
+  end compareErasedGlb
 
   /** Does the (possibly generic) type `tp` have the same erasure in all its
    *  possible instantiations?
@@ -522,7 +572,7 @@ class TypeErasure(sourceLanguage: SourceLanguage, semiEraseVCs: Boolean, isConst
       if sourceLanguage.isScala2 then
         this(Scala2Erasure.intersectionDominator(Scala2Erasure.flattenedParents(tp)))
       else
-        erasedGlb(this(tp1), this(tp2), isJava = sourceLanguage.isJava)
+        erasedGlb(this(tp1), this(tp2))
     case OrType(tp1, tp2) =>
       if isSymbol && sourceLanguage.isScala2 && ctx.settings.scalajs.value then
         // In Scala2Unpickler we unpickle Scala.js pseudo-unions as if they were

compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala

@@ -10,7 +10,7 @@ import core.Flags._
 import core.Names.{DerivedName, Name, SimpleName, TypeName}
 import core.Symbols._
 import core.TypeApplications.TypeParamInfo
-import core.TypeErasure.{erasure, isUnboundedGeneric}
+import core.TypeErasure.{erasedGlb, erasure, isUnboundedGeneric}
 import core.Types._
 import core.classfile.ClassfileConstants
 import ast.Trees._
@@ -19,6 +19,7 @@ import TypeUtils._
 import java.lang.StringBuilder
 
 import scala.annotation.tailrec
+import scala.collection.mutable.ListBuffer
 
 /** Helper object to generate generic java signatures, as defined in
  *  the Java Virtual Machine Specification, §4.3.4
@@ -65,18 +66,79 @@ object GenericSignatures {
 
     def boxedSig(tp: Type): Unit = jsig(tp.widenDealias, primitiveOK = false)
 
+    /** The signature of the upper-bound of a type parameter.
+     *
+     *  @pre none of the bounds are themselves type parameters.
+     *       TODO: Remove this restriction so we can support things like:
+     *
+     *           class Foo[A]:
+     *              def foo[S <: A & Object](...): ...
+     *
+     *        Which should emit a signature `S <: A`. See the handling
+     *        of `AndType` in `jsig` which already supports `def foo(x: A & Object)`.
+     */
     def boundsSig(bounds: List[Type]): Unit = {
-      val (isTrait, isClass) = bounds partition (_.typeSymbol.is(Trait))
-      isClass match {
-        case Nil    => builder.append(':') // + boxedSig(ObjectTpe)
-        case x :: _ => builder.append(':'); boxedSig(x)
-      }
-      isTrait.foreach { tp =>
+      val (repr :: _, others) = splitIntersection(bounds)
+      builder.append(':')
+
+      // According to the Java spec
+      // (https://docs.oracle.com/javase/specs/jls/se8/html/jls-4.html#jls-4.4),
+      // intersections erase to their first member and must start with a class.
+      // So, if our intersection erases to a trait, in theory we should emit
+      // just that trait in the generic signature even if the intersection type
+      // is composed of multiple traits. But in practice Scala 2 has always
+      // ignored this restriction as intersections of traits seem to be handled
+      // correctly by javac, we do the same here since type soundness seems
+      // more important than adhering to the spec.
+      if repr.classSymbol.is(Trait) then
+        builder.append(':')
+        boxedSig(repr)
+        // If we wanted to be compliant with the spec, we would `return` here.
+      else
+        boxedSig(repr)
+      others.filter(_.classSymbol.is(Trait)).foreach { tp =>
         builder.append(':')
         boxedSig(tp)
       }
     }
 
+    /** The parents of this intersection where type parameters
+     *  that cannot appear in the signature have been replaced
+     *  by their upper-bound.
+     */
+    def flattenedIntersection(tp: AndType)(using Context): List[Type] =
+      val parents = ListBuffer[Type]()
+
+      def collect(parent: Type, parents: ListBuffer[Type]): Unit = parent.widenDealias match
+        case AndType(tp1, tp2) =>
+          collect(tp1, parents)
+          collect(tp2, parents)
+        case parent: TypeRef =>
+          if parent.symbol.isClass || isTypeParameterInSig(parent.symbol, sym0) then
+            parents += parent
+          else
+            collect(parent.superType, parents)
+        case parent =>
+          parents += parent
+
+      collect(tp, parents)
+      parents.toList
+    end flattenedIntersection
+
+    /** Split the `parents` of an intersection into two subsets:
+     *  those whose individual erasure matches the overall erasure
+     *  of the intersection and the others.
+     */
+    def splitIntersection(parents: List[Type])(using Context): (List[Type], List[Type]) =
+      val erasedParents = parents.map(erasure)
+      val erasedCls = erasedGlb(erasedParents).classSymbol
+      parents.zip(erasedParents)
+        .partitionMap((parent, erasedParent) =>
+          if erasedParent.classSymbol eq erasedCls then
+            Left(parent)
+          else
+            Right(parent))
+
     def paramSig(param: LambdaParam): Unit = {
       builder.append(sanitizeName(param.paramName))
       boundsSig(hiBounds(param.paramInfo.bounds))
@@ -263,8 +325,20 @@ object GenericSignatures {
           builder.append(')')
           methodResultSig(restpe)
 
-        case AndType(tp1, tp2) =>
-          jsig(intersectionDominator(tp1 :: tp2 :: Nil), primitiveOK = primitiveOK)
+        case tp: AndType =>
+          // Only intersections appearing as the upper-bound of a type parameter
+          // can be preserved in generic signatures and those are already
+          // handled by `boundsSig`, so here we fallback to picking a parent of
+          // the intersection to determine its overall signature. We must pick a
+          // parent whose erasure matches the erasure of the intersection
+          // because javac relies on the generic signature to determine the
+          // bytecode signature. Additionally, we prefer picking a type
+          // parameter since that will likely lead to a more precise type.
+          val parents = flattenedIntersection(tp)
+          val (reprParents, _) = splitIntersection(parents)
+          val repr =
+            reprParents.find(_.typeSymbol.is(TypeParam)).getOrElse(reprParents.head)
+          jsig(repr, primitiveOK)
 
         case ci: ClassInfo =>
           def polyParamSig(tparams: List[TypeParamInfo]): Unit =
@@ -308,38 +382,6 @@ object GenericSignatures {
 
   private class UnknownSig extends Exception
 
-  /** The intersection dominator (SLS 3.7) of a list of types is computed as follows.
-    *
-    *  - If the list contains one or more occurrences of scala.Array with
-    *    type parameters El1, El2, ... then the dominator is scala.Array with
-    *    type parameter of intersectionDominator(List(El1, El2, ...)).           <--- @PP: not yet in spec.
-    *  - Otherwise, the list is reduced to a subsequence containing only the
-    *    types that are not supertypes of other listed types (the span.)
-    *  - If the span is empty, the dominator is Object.
-    *  - If the span contains a class Tc which is not a trait and which is
-    *    not Object, the dominator is Tc.                                        <--- @PP: "which is not Object" not in spec.
-    *  - Otherwise, the dominator is the first element of the span.
-    */
-  private def intersectionDominator(parents: List[Type])(using Context): Type =
-    if (parents.isEmpty) defn.ObjectType
-    else {
-      val psyms = parents map (_.typeSymbol)
-      if (psyms contains defn.ArrayClass)
-        // treat arrays specially
-        defn.ArrayType.appliedTo(intersectionDominator(parents.filter(_.typeSymbol == defn.ArrayClass).map(t => t.argInfos.head)))
-      else {
-        // implement new spec for erasure of refined types.
-        def isUnshadowed(psym: Symbol) =
-          !(psyms exists (qsym => (psym ne qsym) && (qsym isSubClass psym)))
-        val cs = parents.iterator.filter { p => // isUnshadowed is a bit expensive, so try classes first
-          val psym = p.classSymbol
-          psym.ensureCompleted()
-          psym.isClass && !psym.is(Trait) && isUnshadowed(psym)
-        }
-        (if (cs.hasNext) cs else parents.iterator.filter(p => isUnshadowed(p.classSymbol))).next()
-      }
-    }
-
   /* Drop redundant types (ones which are implemented by some other parent) from the immediate parents.
    * This is important on Android because there is otherwise an interface explosion.
    */

sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala

@@ -22,7 +22,7 @@ class Outer {
 // This is enforced by dottyApp/Main.scala
 class Z {
   def a_01(a: A with B): Unit = {}
-  def b_02X(b: B with A): Unit = {}
+  def a_02X(b: B with A): Unit = {}
   def a_02(a: A with B with A): Unit = {}
   def a_03(a: A with (B with A)): Unit = {}
   def a_04(b: A with (B with A) @foo): Unit = {}
@@ -33,15 +33,15 @@ class Z {
   def a_06(a: T1): Unit = {}
 
   type S <: B with T1
-  def b_07(a: S): Unit = {}
+  def a_07(a: S): Unit = {}
 
   type T2 <: B with A
   type U <: T2 with S
-  def b_08(b: U): Unit = {}
+  def a_08(b: U): Unit = {}
 
   val singB: B = new B {}
   def a_09(a: A with singB.type): Unit = {}
-  def b_10(b: singB.type with A): Unit = {}
+  def a_10(b: singB.type with A): Unit = {}
 
   type V >: SubB <: B
   def b_11(b: V): Unit = {}
@@ -71,46 +71,46 @@ class Z {
 
   type W1 <: A with Cov[Any]
   type X1 <: Cov[Int] with W1
-  def cov_21(a: X1): Unit = {}
+  def a_21(a: X1): Unit = {}
 
   type W2 <: A with Cov[Any]
   type X2 <: Cov[Int] with W2
-  def cov_22(a: X2): Unit = {}
+  def a_22(a: X2): Unit = {}
 
   def z_23(z: A with this.type): Unit = {}
   def z_24(z: this.type with A): Unit = {}
 
   def a_25(b: A with (B { type T })): Unit = {}
   def a_26(a: (A { type T }) with ((B with A) { type T })): Unit = {}
 
-  def b_27(a: VC with B): Unit = {}
-  def b_28(a: B with VC): Unit = {}
+  def a_27(a: VC with B): Unit = {}
+  def a_28(a: B with VC): Unit = {}
 
   val o1: Outer = new Outer
   val o2: Outer = new Outer
-  def f_29(f: o1.E with o1.F): Unit = {}
-  def f_30(f: o1.F with o1.E): Unit = {}
-  def f_31(f: o1.E with o2.F): Unit = {}
-  def f_32(f: o2.F with o1.E): Unit = {}
-  def f_33(f: Outer#E with Outer#F): Unit = {}
-  def f_34(f: Outer#F with Outer#E): Unit = {}
+  def e_29(f: o1.E with o1.F): Unit = {}
+  def e_30(f: o1.F with o1.E): Unit = {}
+  def e_31(f: o1.E with o2.F): Unit = {}
+  def e_32(f: o2.F with o1.E): Unit = {}
+  def e_33(f: Outer#E with Outer#F): Unit = {}
+  def e_34(f: Outer#F with Outer#E): Unit = {}
 
   val structural1: { type DSub <: D } = new { type DSub <: D }
   def d_35(a: A with structural1.DSub): Unit = {}
   def d_36(a: structural1.DSub with A): Unit = {}
-  def z_37(z: Z with structural1.DSub): Unit = {}
+  def d_37(z: Z with structural1.DSub): Unit = {}
   def d_38(z: structural1.DSub with Z): Unit = {}
 
   val structural2: { type SubCB <: C with B } = new { type SubCB <: C with B }
-  def c_39(c: structural2.SubCB with B): Unit = {}
+  def b_39(c: structural2.SubCB with B): Unit = {}
   def b_40(c: B with structural2.SubCB): Unit = {}
 
   val structural3a: { type SubB <: B; type SubCB <: C with SubB } = new { type SubB <: B; type SubCB <: C with SubB }
   val structural3b: { type SubB <: B; type SubCB <: C with SubB } = new { type SubB <: B; type SubCB <: C with SubB }
   def b_41(c: structural3a.SubB with structural3a.SubCB): Unit = {}
-  def c_42(c: structural3a.SubCB with structural3a.SubB): Unit = {}
+  def b_42(c: structural3a.SubCB with structural3a.SubB): Unit = {}
   def b_43(b: structural3a.SubB with structural3b.SubCB): Unit = {}
-  def c_44(c: structural3b.SubCB with structural3a.SubB): Unit = {}
+  def b_44(c: structural3b.SubCB with structural3a.SubB): Unit = {}
 
   type SubStructural <: C with structural3a.SubB
   def b_45(x: structural3a.SubB with SubStructural): Unit = {}

tests/neg/i5139.scala

@@ -0,0 +1,7 @@
+trait A
+trait B
+
+class Overload {
+  def m(ab: A&B) = ab
+  def m(ab: B&A) = ab // error: double definition
+}

tests/neg/ostermann.scala

@@ -0,0 +1,5 @@
+trait A { def foo(a: A) : Unit }
+
+trait C extends A {
+  override def foo(a: A with Any): Unit // error: method foo has a different signature than the overridden declaration
+}