diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index b8d1caf3d1b7..d97ae8ca2f6e 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -1021,7 +1021,7 @@ object Denotations { * erasure (see i8615b, i9109b), Erasure takes care of adding any necessary * bridge to make this work at runtime. */ - def matchesLoosely(other: SingleDenotation)(using Context): Boolean = + def matchesLoosely(other: SingleDenotation, alwaysCompareTypes: Boolean = false)(using Context): Boolean = if isType then true else val thisLanguage = SourceLanguage(symbol) @@ -1031,7 +1031,7 @@ object Denotations { val otherSig = other.signature(commonLanguage) sig.matchDegree(otherSig) match case FullMatch => - true + !alwaysCompareTypes || info.matches(other.info) case MethodNotAMethodMatch => !ctx.erasedTypes && { // A Scala zero-parameter method and a Scala non-method always match. diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index 8cb78c258704..cc4147d7d622 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -36,7 +36,7 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { !sym.isOneOf(MethodOrModule) || super.exclude(sym) } - //val site = root.thisType + val site = root.thisType private var toBeRemoved = immutable.Set[Symbol]() private val bridges = mutable.ListBuffer[Tree]() @@ -77,7 +77,13 @@ class Bridges(root: ClassSymbol, thisPhase: DenotTransformer)(using Context) { |clashes with definition of the member itself; both have erased type ${info(member)(using elimErasedCtx)}."""", bridgePosFor(member)) } - else if (!bridgeExists) + else if !inContext(preErasureCtx)(site.memberInfo(member).matches(site.memberInfo(other))) then + // Neither symbol signatures nor pre-erasure types seen from root match; this means + // according to Scala 2 semantics there is no override. + // A bridge might introduce a classcast exception. + // Example where this was observed: run/i12828a.scala and MapView in stdlib213 + report.log(i"suppress bridge in $root for ${member} in ${member.owner} and ${other.showLocated} since member infos ${site.memberInfo(member)} and ${site.memberInfo(other)} do not match") + else if !bridgeExists then addBridge(member, other) } diff --git a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala index b2851fd92619..6ccb06a2f9a6 100644 --- a/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala +++ b/compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala @@ -1,8 +1,9 @@ -package dotty.tools.dotc +package dotty.tools +package dotc package transform import core._ -import Flags._, Symbols._, Contexts._, Scopes._, Decorators._ +import Flags._, Symbols._, Contexts._, Scopes._, Decorators._, Types.Type import collection.mutable import collection.immutable.BitSet import scala.annotation.tailrec @@ -33,11 +34,11 @@ object OverridingPairs { * pair has already been treated in a parent class. * This may be refined in subclasses. @see Bridges for a use case. */ - protected def parents: Array[Symbol] = base.info.parents.toArray.map(_.typeSymbol) + protected def parents: Array[Symbol] = base.info.parents.toArray.map(_.classSymbol) - /** Does `sym1` match `sym2` so that it qualifies as overriding. - * Types always match. Term symbols match if their membertypes - * relative to .this do + /** Does `sym1` match `sym2` so that it qualifies as overriding when both symbols are + * seen as members of `self`? Types always match. Term symbols match if their membertypes + * relative to `self` do. */ protected def matches(sym1: Symbol, sym2: Symbol): Boolean = sym1.isType || sym1.asSeenFrom(self).matches(sym2.asSeenFrom(self)) @@ -85,11 +86,22 @@ object OverridingPairs { then bits += i subParents(bc) = bits - private def hasCommonParentAsSubclass(cls1: Symbol, cls2: Symbol): Boolean = - (subParents(cls1) intersect subParents(cls2)).nonEmpty + /** Is the override of `sym1` and `sym2` already handled when checking + * a parent of `self`? + */ + private def isHandledByParent(sym1: Symbol, sym2: Symbol): Boolean = + val commonParents = subParents(sym1.owner).intersect(subParents(sym2.owner)) + commonParents.nonEmpty + && commonParents.exists(i => canBeHandledByParent(sym1, sym2, parents(i))) + + /** Can pair `sym1`/`sym2` be handled by parent `parentType` which is a common subtype + * of both symbol's owners? Assumed to be true by default, but overridden in RefChecks. + */ + protected def canBeHandledByParent(sym1: Symbol, sym2: Symbol, parent: Symbol): Boolean = + true /** The scope entries that have already been visited as overridden - * (maybe excluded because of hasCommonParentAsSubclass). + * (maybe excluded because of already handled by a parent). * These will not appear as overriding */ private val visited = util.HashSet[Symbol]() @@ -134,28 +146,22 @@ object OverridingPairs { * overridden = overridden member of the pair, provided hasNext is true */ @tailrec final def next(): Unit = - if (nextEntry ne null) { + if nextEntry != null then nextEntry = decls.lookupNextEntry(nextEntry) - if (nextEntry ne null) - try { + if nextEntry != null then + try overridden = nextEntry.sym - if (overriding.owner != overridden.owner && matches(overriding, overridden)) { + if overriding.owner != overridden.owner && matches(overriding, overridden) then visited += overridden - if (!hasCommonParentAsSubclass(overriding.owner, overridden.owner)) return - } - } - catch { - case ex: TypeError => - // See neg/i1750a for an example where a cyclic error can arise. - // The root cause in this example is an illegal "override" of an inner trait - report.error(ex, base.srcPos) - } - else { + if !isHandledByParent(overriding, overridden) then return + catch case ex: TypeError => + // See neg/i1750a for an example where a cyclic error can arise. + // The root cause in this example is an illegal "override" of an inner trait + report.error(ex, base.srcPos) + else curEntry = curEntry.prev nextOverriding() - } next() - } nextOverriding() next() diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 976607c79f1e..af33ca2ff5c2 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -323,6 +323,14 @@ object RefChecks { overrideErrorMsg("no longer has compatible type"), (if (member.owner == clazz) member else clazz).srcPos)) + /** 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 + * error messages. Also, bridges are not generated in this case. + * Type members are always assumed to match. + */ + def trueMatch: Boolean = + member.isType || memberTp(self).matches(otherTp(self)) + def emitOverrideError(fullmsg: Message) = if (!(hasErrors && member.is(Synthetic) && member.is(Module))) { // suppress errors relating toi synthetic companion objects if other override @@ -333,7 +341,7 @@ object RefChecks { } def overrideError(msg: String, compareTypes: Boolean = false) = - if (noErrorType) + if trueMatch && noErrorType then emitOverrideError(overrideErrorMsg(msg, compareTypes)) def autoOverride(sym: Symbol) = @@ -360,24 +368,6 @@ object RefChecks { //Console.println(infoString(member) + " overrides " + infoString(other) + " in " + clazz);//DEBUG - // return if we already checked this combination elsewhere - if (member.owner != clazz) { - def deferredCheck = member.is(Deferred) || !other.is(Deferred) - def subOther(s: Symbol) = s derivesFrom other.owner - def subMember(s: Symbol) = s derivesFrom member.owner - - if (subOther(member.owner) && deferredCheck) - //Console.println(infoString(member) + " shadows1 " + infoString(other) " in " + clazz);//DEBUG - return - val parentSymbols = clazz.info.parents.map(_.typeSymbol) - if (parentSymbols exists (p => subOther(p) && subMember(p) && deferredCheck)) - //Console.println(infoString(member) + " shadows2 " + infoString(other) + " in " + clazz);//DEBUG - return - if (parentSymbols forall (p => subOther(p) == subMember(p))) - //Console.println(infoString(member) + " shadows " + infoString(other) + " in " + clazz);//DEBUG - return - } - /* Is the intersection between given two lists of overridden symbols empty? */ def intersectionIsEmpty(syms1: Iterator[Symbol], syms2: Iterator[Symbol]) = { val set2 = syms2.toSet @@ -412,7 +402,7 @@ object RefChecks { overrideError("cannot be used here - class definitions cannot be overridden") else if (!other.is(Deferred) && member.isClass) overrideError("cannot be used here - classes can only override abstract types") - else if (other.isEffectivelyFinal) // (1.2) + else if other.isEffectivelyFinal then // (1.2) overrideError(i"cannot override final member ${other.showLocated}") else if (member.is(ExtensionMethod) && !other.is(ExtensionMethod)) // (1.3) overrideError("is an extension method, cannot override a normal method") @@ -433,9 +423,11 @@ object RefChecks { member.setFlag(Override) else if (member.isType && self.memberInfo(member) =:= self.memberInfo(other)) () // OK, don't complain about type aliases which are equal - else if (member.owner != clazz && other.owner != clazz && - !(other.owner derivesFrom member.owner)) - emitOverrideError( + else if member.owner != clazz + && other.owner != clazz + && !other.owner.derivesFrom(member.owner) + then + overrideError( s"$clazz inherits conflicting members:\n " + infoStringWithLocation(other) + " and\n " + infoStringWithLocation(member) + "\n(Note: this can be resolved by declaring an override in " + clazz + ".)") @@ -496,25 +488,51 @@ object RefChecks { }*/ } - val opc = new OverridingPairs.Cursor(clazz): - - /** 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 - */ - override def matches(sym1: Symbol, sym2: Symbol): Boolean = - !(sym1.owner.is(JavaDefined, butNot = Trait) && sym2.owner.is(JavaDefined, butNot = Trait)) && // javac already handles these checks - (sym1.isType || { - val sd1 = sym1.asSeenFrom(clazz.thisType) - val sd2 = sym2.asSeenFrom(clazz.thisType) - sd1.matchesLoosely(sd2) + /** 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) + + private def inLinearizationOrder(sym1: Symbol, sym2: Symbol, parent: Symbol): Boolean = + val owner1 = sym1.owner + val owner2 = sym2.owner + def precedesIn(bcs: List[ClassSymbol]): Boolean = (bcs: @unchecked) match + case bc :: bcs1 => + if owner1 eq bc then true + else if owner2 eq bc then false + else precedesIn(bcs1) + case _ => + false + precedesIn(parent.asClass.baseClasses) + + // We can exclude pairs safely from checking only under two additional conditions + // - their signatures also match in the parent class. + // See neg/i12828.scala for an example where this matters. + // - 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) + .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 while opc.hasNext do @@ -528,13 +546,11 @@ object RefChecks { // // class A { type T = B } // class B extends A { override type T } - for - dcl <- clazz.info.decls.iterator - if dcl.is(Deferred) - other <- dcl.allOverriddenSymbols - if !other.is(Deferred) - do - checkOverride(dcl, other) + for dcl <- clazz.info.decls.iterator do + if dcl.is(Deferred) then + for other <- dcl.allOverriddenSymbols do + if !other.is(Deferred) then + checkOverride(dcl, other) printMixinOverrideErrors() @@ -578,7 +594,8 @@ object RefChecks { def isConcrete(sym: Symbol) = sym.exists && !sym.isOneOf(NotConcrete) clazz.nonPrivateMembersNamed(mbr.name) .filterWithPredicate( - impl => isConcrete(impl.symbol) && mbrDenot.matchesLoosely(impl)) + impl => isConcrete(impl.symbol) + && mbrDenot.matchesLoosely(impl, alwaysCompareTypes = true)) .exists /** The term symbols in this class and its baseclasses that are diff --git a/tests/neg/OpaqueEscape.scala b/tests/neg/OpaqueEscape.scala index a6f701fd4e3b..1596003d4244 100644 --- a/tests/neg/OpaqueEscape.scala +++ b/tests/neg/OpaqueEscape.scala @@ -7,7 +7,7 @@ def unwrap(i:Wrapped):Int def wrap(i:Int):Wrapped } class Escaper extends EscaperBase{ // error: needs to be abstract - override def unwrap(i:Int):Int = i // error overriding method unwrap + override def unwrap(i:Int):Int = i // was error overriding method unwrap, now OK override def wrap(i:Int):Int = i // error overriding method wrap } val e = new Escaper:EscaperBase diff --git a/tests/neg/i11828.check b/tests/neg/i11828.check new file mode 100644 index 000000000000..80824e9b1334 --- /dev/null +++ b/tests/neg/i11828.check @@ -0,0 +1,8 @@ +-- Error: tests/neg/i12828.scala:7:7 ----------------------------------------------------------------------------------- +7 |object Baz extends Bar[Int] // error overriding foo: incompatible type + | ^ + | object creation impossible, since def foo(x: A): Unit in trait Foo is not defined + | (Note that + | parameter A in def foo(x: A): Unit in trait Foo does not match + | parameter Int & String in def foo(x: A & String): Unit in trait Bar + | ) diff --git a/tests/neg/i12828.check b/tests/neg/i12828.check new file mode 100644 index 000000000000..070633fc35b3 --- /dev/null +++ b/tests/neg/i12828.check @@ -0,0 +1,8 @@ +-- Error: tests/neg/i12828.scala:7:7 ----------------------------------------------------------------------------------- +7 |object Baz extends Bar[Int] // error: not implemented + | ^ + | object creation impossible, since def foo(x: A): Unit in trait Foo is not defined + | (Note that + | parameter A in def foo(x: A): Unit in trait Foo does not match + | parameter Int & String in def foo(x: A & String): Unit in trait Bar + | ) diff --git a/tests/neg/i12828.scala b/tests/neg/i12828.scala new file mode 100644 index 000000000000..d8d099b71d08 --- /dev/null +++ b/tests/neg/i12828.scala @@ -0,0 +1,9 @@ +trait Foo[A]: + def foo(x: A): Unit + +trait Bar[A] extends Foo[A]: + def foo(x: A & String): Unit = println(x.toUpperCase) + +object Baz extends Bar[Int] // error: not implemented + +@main def run() = Baz.foo(42) diff --git a/tests/neg/i12828c.scala b/tests/neg/i12828c.scala new file mode 100644 index 000000000000..d36e7a719984 --- /dev/null +++ b/tests/neg/i12828c.scala @@ -0,0 +1,12 @@ +abstract class Foo[A] { + def foo(x: A): Unit +} +abstract class Bar[A] extends Foo[A] { + def foo(x: A with String): Unit = println(x.toUpperCase) +} +object Baz extends Bar[Int] // error: not implemented (same as Scala 2) + // Scala 2 gives: object creation impossible. Missing implementation for `foo` + +object Test { + def main(args: Array[String]) = Baz.foo(42) +} diff --git a/tests/neg/i12828d.scala b/tests/neg/i12828d.scala new file mode 100644 index 000000000000..45a95501835d --- /dev/null +++ b/tests/neg/i12828d.scala @@ -0,0 +1,18 @@ +trait A[X] { + def foo(x: X): Unit = + println("A.foo") +} +trait B[X] extends A[X] { + def foo(x: Int): Unit = + println("B.foo") +} +object C extends B[Int] // error: conflicting members + // Scala 2: same + +object Test { + def main(args: Array[String]) = { + C.foo(1) + val a: A[Int] = C + a.foo(1) + } +} \ No newline at end of file diff --git a/tests/run/i5094.scala b/tests/neg/i5094.scala similarity index 81% rename from tests/run/i5094.scala rename to tests/neg/i5094.scala index 556a1f0f07df..755e81addf09 100644 --- a/tests/run/i5094.scala +++ b/tests/neg/i5094.scala @@ -9,7 +9,7 @@ trait SOIO extends IO { } trait SOSO extends SOIO with SO abstract class AS extends SO -class L extends AS with SOSO +class L extends AS with SOSO // error: cannot override final member object Test { def main(args: Array[String]): Unit = { new L diff --git a/tests/neg/i7597.scala b/tests/neg/i7597.scala index 8b18b82b1db4..cc41a3c77e2d 100644 --- a/tests/neg/i7597.scala +++ b/tests/neg/i7597.scala @@ -6,8 +6,8 @@ object Test extends App { def apply(x: A): B } - class C[S <: String] extends Fn[String, Int] { - def apply(s: S): Int = 0 // error + class C[S <: String] extends Fn[String, Int] { // error + def apply(s: S): Int = 0 } foo("") diff --git a/tests/neg/varargs-annot-2.scala b/tests/neg/varargs-annot-2.scala new file mode 100644 index 000000000000..7b0fce1124ea --- /dev/null +++ b/tests/neg/varargs-annot-2.scala @@ -0,0 +1,9 @@ +import annotation.varargs + +trait C { + @varargs def v(i: Int*) = () +} + +class D extends C { // error: name clash between defined and inherited member + def v(i: Array[Int]) = () +} \ No newline at end of file diff --git a/tests/neg/varargs-annot.scala b/tests/neg/varargs-annot.scala index 490d2ab93695..d24b9d350d78 100644 --- a/tests/neg/varargs-annot.scala +++ b/tests/neg/varargs-annot.scala @@ -17,7 +17,7 @@ object Test { class D extends C { override def v(i: Int*) = () // error - def v(i: Array[Int]) = () // error + def v(i: Array[Int]) = () // ok, reported when used alone (see varargs-annot-2.scala) } @varargs def nov(a: Int) = 0 // error: A method without repeated parameters cannot be annotated with @varargs diff --git a/tests/run/i12828a.scala b/tests/run/i12828a.scala new file mode 100644 index 000000000000..459af156980c --- /dev/null +++ b/tests/run/i12828a.scala @@ -0,0 +1,12 @@ +trait Foo[A] { + def foo(x: A): Unit = () +} +trait Bar[A] extends Foo[A] { + def foo(x: A with String): Unit = println(x.toUpperCase) +} +object Baz extends Bar[Int] // was: error: Baz inherits conflicting members, now like Scala 2 + // Scala 2 compiles and runs + +object Test { + def main(args: Array[String]) = Baz.foo(42) +} diff --git a/tests/run/i12828b.scala b/tests/run/i12828b.scala new file mode 100644 index 000000000000..d1cd3e087a36 --- /dev/null +++ b/tests/run/i12828b.scala @@ -0,0 +1,12 @@ +class Foo[A] { + def foo(x: A): Unit = () +} +class Bar[A] extends Foo[A] { + def foo(x: A with String): Unit = println(x.toUpperCase) +} +object Baz extends Bar[Int] // was error: Baz inherits conflicting members, now like Scalac + // Scala 2 compiles and runs + +object Test { + def main(args: Array[String]) = Baz.foo(42) +}