diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index cc4147d7d622..d91840f247b6 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -34,6 +34,9 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { override def exclude(sym: Symbol) = !sym.isOneOf(MethodOrModule) || super.exclude(sym) + + override def canBeHandledByParent(sym1: Symbol, sym2: Symbol, parent: Symbol): Boolean = + OverridingPairs.isOverridingPair(sym1, sym2, parent.thisType) } val site = root.thisType diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index 6ccb06a2f9a6..437dfea9f156 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -4,6 +4,7 @@ package transform import core._ import Flags._, Symbols._, Contexts._, Scopes._, Decorators._, Types.Type +import NameKinds.DefaultGetterName import collection.mutable import collection.immutable.BitSet import scala.annotation.tailrec @@ -16,12 +17,12 @@ import scala.annotation.tailrec * Adapted from the 2.9 version of OverridingPairs. The 2.10 version is IMO * way too unwieldy to be maintained. */ -object OverridingPairs { +object OverridingPairs: /** The cursor class * @param base the base class that contains the overriding pairs */ - class Cursor(base: Symbol)(using Context) { + class Cursor(base: Symbol)(using Context): private val self = base.thisType @@ -165,5 +166,64 @@ object OverridingPairs { nextOverriding() next() - } -} + end Cursor + + /** Is this `sym1` considered an override of `sym2` (or vice versa) if both are + * seen as members of `site`? + * We declare a match if either we have a full match including matching names + * or we have a loose match with different target name but the types are the same. + * We leave out pairs of methods in Java classes under the assumption since these + * have already been checked and handled by javac. + * This leaves two possible sorts of discrepancies to be reported as errors + * in `RefChecks`: + * + * - matching names, target names, and signatures but different types + * - matching names and types, but different target names + * + * This method is used as a replacement of `matches` in some subclasses of + * OverridingPairs. + */ + def isOverridingPair(sym1: Symbol, sym2: Symbol, self: Type)(using Context): Boolean = + if sym1.owner.is(JavaDefined, butNot = Trait) + && sym2.owner.is(JavaDefined, butNot = Trait) + then false // javac already handles these checks and inserts bridges + else if sym1.isType then true + else + val sd1 = sym1.asSeenFrom(self) + val sd2 = sym2.asSeenFrom(self) + sd1.matchesLoosely(sd2) + && (sym1.hasTargetName(sym2.targetName) + || isOverridingPair(sym1, sd1.info, sym2, sd2.info)) + + /** Let `member` and `other` be members of some common class C with types + * `memberTp` and `otherTp` in C. Are the two symbols considered an overriding + * pair in C? We assume that names already match so we test only the types here. + * @param fallBack A function called if the initial test is false and + * `member` and `other` are term symbols. + */ + def isOverridingPair(member: Symbol, memberTp: Type, other: Symbol, otherTp: Type, fallBack: => Boolean = false)(using Context): Boolean = + if member.isType then // intersection of bounds to refined types must be nonempty + memberTp.bounds.hi.hasSameKindAs(otherTp.bounds.hi) + && ( + (memberTp frozen_<:< otherTp) + || !member.owner.derivesFrom(other.owner) + && { + // if member and other come from independent classes or traits, their + // bounds must have non-empty-intersection + val jointBounds = (memberTp.bounds & otherTp.bounds).bounds + jointBounds.lo frozen_<:< jointBounds.hi + } + ) + else + // releaxed override check for explicit nulls if one of the symbols is Java defined, + // force `Null` being a subtype of reference types during override checking + val relaxedCtxForNulls = + if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then + ctx.retractMode(Mode.SafeNulls) + else ctx + member.name.is(DefaultGetterName) // default getters are not checked for compatibility + || memberTp.overrides(otherTp, + member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack + )(using relaxedCtxForNulls) + +end OverridingPairs diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index af33ca2ff5c2..37bdde5b0559 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -17,6 +17,7 @@ import config.Printers.{checks, noPrinter} import scala.util.{Try, Failure, Success} import config.{ScalaVersion, NoScalaVersion} import Decorators._ +import OverridingPairs.isOverridingPair import typer.ErrorReporting._ import config.Feature.{warnOnMigration, migrateTo3} import config.Printers.refcheck @@ -264,31 +265,6 @@ object RefChecks { i"${if (showLocation) sym1.showLocated else sym1}$infoStr" } - def compatibleTypes(member: Symbol, memberTp: Type, other: Symbol, otherTp: Type, fallBack: => Boolean = false): Boolean = - try - if (member.isType) // intersection of bounds to refined types must be nonempty - memberTp.bounds.hi.hasSameKindAs(otherTp.bounds.hi) && - ((memberTp frozen_<:< otherTp) || - !member.owner.derivesFrom(other.owner) && { - // if member and other come from independent classes or traits, their - // bounds must have non-empty-intersection - val jointBounds = (memberTp.bounds & otherTp.bounds).bounds - jointBounds.lo frozen_<:< jointBounds.hi - }) - else - // releaxed override check for explicit nulls if one of the symbols is Java defined, - // force `Null` being a subtype of reference types during override checking - val relaxedCtxForNulls = - if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then - ctx.retractMode(Mode.SafeNulls) - else ctx - member.name.is(DefaultGetterName) // default getters are not checked for compatibility - || memberTp.overrides(otherTp, member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack)(using relaxedCtxForNulls) - catch case ex: MissingType => - // can happen when called with upwardsSelf as qualifier of memberTp and otherTp, - // because in that case we might access types that are not members of the qualifier. - false - /* Check that all conditions for overriding `other` by `member` * of class `clazz` are met. */ @@ -318,10 +294,15 @@ object RefChecks { } def compatTypes(memberTp: Type, otherTp: Type): Boolean = - compatibleTypes(member, memberTp, other, otherTp, - fallBack = warnOnMigration( - overrideErrorMsg("no longer has compatible type"), - (if (member.owner == clazz) member else clazz).srcPos)) + try + isOverridingPair(member, memberTp, other, otherTp, + fallBack = warnOnMigration( + overrideErrorMsg("no longer has compatible type"), + (if (member.owner == clazz) member else clazz).srcPos)) + catch case ex: MissingType => + // can happen when called with upwardsSelf as qualifier of memberTp and otherTp, + // because in that case we might access types that are not members of the qualifier. + false /** Do types of term members `member` and `other` as seen from `self` match? * If not we treat them as not a real override and don't issue override @@ -488,29 +469,9 @@ object RefChecks { }*/ } - /** We declare a match if either we have a full match including matching names - * or we have a loose match with different target name but the types are the same. - * This leaves two possible sorts of discrepancies to be reported as errors - * in `checkOveride`: - * - * - matching names, target names, and signatures but different types - * - matching names and types, but different target names - */ - def considerMatching(sym1: Symbol, sym2: Symbol, self: Type): Boolean = - if sym1.owner.is(JavaDefined, butNot = Trait) - && sym2.owner.is(JavaDefined, butNot = Trait) - then false // javac already handles these checks - else if sym1.isType then true - else - val sd1 = sym1.asSeenFrom(self) - val sd2 = sym2.asSeenFrom(self) - sd1.matchesLoosely(sd2) - && (sym1.hasTargetName(sym2.targetName) - || compatibleTypes(sym1, sd1.info, sym2, sd2.info)) - val opc = new OverridingPairs.Cursor(clazz): override def matches(sym1: Symbol, sym2: Symbol): Boolean = - considerMatching(sym1, sym2, self) + isOverridingPair(sym1, sym2, self) private def inLinearizationOrder(sym1: Symbol, sym2: Symbol, parent: Symbol): Boolean = val owner1 = sym1.owner @@ -530,7 +491,7 @@ object RefChecks { // - They overriding/overridden appear in linearization order. // See neg/i5094.scala for an example where this matters. override def canBeHandledByParent(sym1: Symbol, sym2: Symbol, parent: Symbol): Boolean = - considerMatching(sym1, sym2, parent.thisType) + isOverridingPair(sym1, sym2, parent.thisType) .showing(i"already handled ${sym1.showLocated}: ${sym1.asSeenFrom(parent.thisType).signature}, ${sym2.showLocated}: ${sym2.asSeenFrom(parent.thisType).signature} = $result", refcheck) && inLinearizationOrder(sym1, sym2, parent) end opc diff --git a/tests/run/i1240.scala b/tests/run/i1240.scala index 7092d91314e4..b425c4f55ff2 100644 --- a/tests/run/i1240.scala +++ b/tests/run/i1240.scala @@ -19,7 +19,9 @@ object Test { // In Java, this gives an error like this: // methods foo(A) from C[D] and foo(String) from C[D] are inherited with the same signature // But the analogous example with `b1` compiles OK in Java. - assert(b2.foo(new D) == "T foo") + assert(b2.foo(new D) == "D foo") + // Here we get "D foo" since a bridge method for foo(x: D) was inserted + // in the anonymous class of b2. } } diff --git a/tests/run/i13087.scala b/tests/run/i13087.scala new file mode 100644 index 000000000000..74eaa0e69e85 --- /dev/null +++ b/tests/run/i13087.scala @@ -0,0 +1,38 @@ +import scala.collection.mutable.Builder + +class DDD[S,T,A] + +trait NN[S, T, A, K[_], +D <: DDD[Set[S],T,K[A]]] +class NNN[S, T, K[_], A] extends NN[S, T, A, K, DDD[Set[S],T,K[A]]] + +object NN { + def newBuilder[S, T, A, K[_]]: + NNbuilder[S, T, K, A, DDD[Set[S],T,K[A]], NN[S,T,A,K,?], Unit] = + new NNNbuilder[S, T, K, A] +} + +// Remove the type parameter E, hardcoding in E := Unit, and the issue +// goes away. +trait NNbuilder + [S, T, K[_], A, +D <: DDD[Set[S],T,K[A]], +N <: NN[S,T,A,K,D], E] + extends Builder[Unit, N] { + def clear(): Unit = throw new UnsupportedOperationException() + final def addOne(builder: E): this.type = this +} + +// Unfold this class defn, and the issue goes away +abstract class AbstractNNNbuilder + [S, T, K[_], A, +D <: DDD[Set[S],T,K[A]], +N <: NN[S,T,A,K,D], E] + extends NNbuilder[S,T,K,A,D,N,E] + +class NNNbuilder[S, T, K[_], A] extends AbstractNNNbuilder[ + S, T, K, A, DDD[Set[S], T, K[A]], NNN[S, T, K, A], Unit +] { + override def result(): NNN[S, T, K, A] = new NNN[S, T, K, A] +} + +@main def Test: Unit = { + val builder = NN.newBuilder[String, Char, Int, Set] + builder += () + builder.result() +} \ No newline at end of file diff --git a/tests/run/i13087a.scala b/tests/run/i13087a.scala new file mode 100644 index 000000000000..f1513583e8fb --- /dev/null +++ b/tests/run/i13087a.scala @@ -0,0 +1,17 @@ +trait SimpleTrait[T] { + def myMethod(t: T): Int + def doIt(t: T): Unit = { + myMethod(t) // java.lang.AbstractMethodError: Method BadClass.myMethod(Ljava/lang/Object;)I is abstract + } +} + +abstract class SimpleClass[T] extends SimpleTrait[T] { + def myMethod(t: String): Int = 5 +} + +class BadClass extends SimpleClass[String] + +object Test { + def main(args: Array[String]): Unit = + (new BadClass).doIt("foobar") +} \ No newline at end of file