Associative, commutative, Java-friendly intersection erasure

Thanks to #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 #5139.

Author: smarter

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 =

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
+}

tests/pos/t8300-overloading.scala renamed to tests/neg/t8300-overloading.scala

@@ -1,4 +1,3 @@
-// cf. neg/t8300-overloading.scala
 trait Universe {
   type Name >: Null <: AnyRef with NameApi
   trait NameApi
@@ -12,5 +11,5 @@ object Test extends App {
   import u.*
 
   def foo(name: Name) = ???
-  def foo(name: TermName) = ???
+  def foo(name: TermName) = ??? // error: double definition, same type after erasure
 }

tests/pos/i5139.scala

@@ -0,0 +1,9 @@
+trait A
+trait B
+
+class Base{
+  def m(ab: A&B) = ab
+}
+class Derived extends Base{
+  override def m(ab: B&A) = ab // unexpected error
+}

tests/pos/ostermann.scala

@@ -0,0 +1,18 @@
+trait One
+trait Two
+
+class X
+class Y extends X
+
+trait YSub extends Y
+
+class A {
+  def foo1[T <: Object with One](x: T): Unit = {}
+  def foo2[T <: One with Object](x: T): Unit = {}
+  def foo3[T <: One with Two](x: T): Unit = {}
+  def foo4[T <: Two with One](x: T): Unit = {}
+  def foo5[T <: X with Y](x: T): Unit = {}
+  def foo6[T <: Y with X](x: T): Unit = {}
+  def foo7[T <: X with YSub](x: T): Unit = {}
+  def foo8[T <: YSub with X](x: T): Unit = {}
+}