Merge pull request #11808 from dotty-staging/new-intersection-erasure

Associative, commutative, Java-friendly intersection erasure

Author: smarter

community-build/community-projects/scala-stm

@@ -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/TypeComparer.scala

@@ -394,32 +394,93 @@ 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 acts 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
+   *  - primitives <= non-primitives
+   *  - real classes <= traits
+   *  - subtypes <= supertypes
+   *
+   *  Since this isn't enough to order to unrelated classes, we use
+   *  lexicographic ordering of the class symbol full name as a tie-breaker.
+   *  This ensure that `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 isPrimitive1 = sym1.isPrimitiveValueClass
+    val isPrimitive2 = sym2.isPrimitiveValueClass
+    if isPrimitive1 && isPrimitive2 then
+      return compareClasses
+    else if isPrimitive1 then
+      return -1
+    else if isPrimitive2 then
+      return 1
+
+    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 +583,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/core/TypeErasure.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 = 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.
    */