diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 33dba2c18ba3..23cd8b25d251 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/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: diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index 707ed053f02a..b855bdfb67e0 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/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 diff --git a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala b/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala index 998dc2fcf9ab..ecaf14b83b4e 100644 --- a/compiler/src/dotty/tools/dotc/transform/GenericSignatures.scala +++ b/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 = diff --git a/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala b/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala index e665b1e4dfb8..66a627f3a352 100644 --- a/sbt-dotty/sbt-test/scala2-compat/erasure/dottyApp/Api.scala +++ b/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,11 +71,11 @@ 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 = {} @@ -83,34 +83,34 @@ class Z { 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 = {} diff --git a/tests/neg/i5139.scala b/tests/neg/i5139.scala new file mode 100644 index 000000000000..bc9fe5867342 --- /dev/null +++ b/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 +} diff --git a/tests/neg/ostermann.scala b/tests/neg/ostermann.scala new file mode 100644 index 000000000000..588841dc9cf4 --- /dev/null +++ b/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 +} diff --git a/tests/pos/t8300-overloading.scala b/tests/neg/t8300-overloading.scala similarity index 75% rename from tests/pos/t8300-overloading.scala rename to tests/neg/t8300-overloading.scala index 61d8df3c5151..e7c829510fcc 100644 --- a/tests/pos/t8300-overloading.scala +++ b/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 } diff --git a/tests/pos/i5139.scala b/tests/pos/i5139.scala new file mode 100644 index 000000000000..f98f8a13ac76 --- /dev/null +++ b/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 +} diff --git a/tests/pos/ostermann.scala b/tests/pos/ostermann.scala deleted file mode 100644 index f3acafc727a9..000000000000 --- a/tests/pos/ostermann.scala +++ /dev/null @@ -1,3 +0,0 @@ -trait A { def foo(a: A) : Unit } - -trait C extends A { override def foo(a: A with Any) : Unit } diff --git a/tests/run/t12300/A_1.scala b/tests/run/t12300/A_1.scala new file mode 100644 index 000000000000..cd664f37fa90 --- /dev/null +++ b/tests/run/t12300/A_1.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 = {} +} diff --git a/tests/run/t12300/B_2.java b/tests/run/t12300/B_2.java new file mode 100644 index 000000000000..d1c42b318570 --- /dev/null +++ b/tests/run/t12300/B_2.java @@ -0,0 +1,17 @@ +class OneTwo implements One, Two {} +// Java doesn't seen class parents of traits +class YSubX extends Y implements YSub {} + +public class B_2 { + public static void foo() { + A a = new A(); + a.foo1(new One() {}); + a.foo2(new One() {}); + a.foo3(new OneTwo()); + a.foo4(new OneTwo()); + a.foo5(new Y()); + a.foo6(new Y()); + a.foo7(new YSubX()); + a.foo8(new YSubX()); + } +} diff --git a/tests/run/t12300/Test_2.scala b/tests/run/t12300/Test_2.scala new file mode 100644 index 000000000000..1aa121dda490 --- /dev/null +++ b/tests/run/t12300/Test_2.scala @@ -0,0 +1,4 @@ +object Test { + def main(args: Array[String]): Unit = + B_2.foo() +}