Skip to content

Add overriding check #11133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,11 @@ object RefChecks {
overrideError("is an extension method, cannot override a normal method")
else if (other.is(ExtensionMethod) && !member.is(ExtensionMethod)) // (1.3)
overrideError("is a normal method, cannot override an extension method")
else if (!other.is(Deferred) &&
!other.name.is(DefaultGetterName) &&
!member.isAnyOverride)
else if !other.is(Deferred)
&& !member.is(Deferred)
&& !other.name.is(DefaultGetterName)
&& !member.isAnyOverride
then
// Exclusion for default getters, fixes SI-5178. We cannot assign the Override flag to
// the default getter: one default getter might sometimes override, sometimes not. Example in comment on ticket.
// Also exclusion for implicit shortcut methods
Expand Down Expand Up @@ -485,6 +487,22 @@ object RefChecks {
while opc.hasNext do
checkOverride(opc.overriding, opc.overridden)
opc.next()

// The OverridingPairs cursor does assume that concrete overrides abstract
// We have to check separately for an abstract definition in a subclass that
// overrides a concrete definition in a superclass. E.g. the following (inspired
// from neg/i11130.scala) needs to be rejected as well:
//
// 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)

printMixinOverrideErrors()

// Verifying a concrete class has nothing unimplemented.
Expand Down Expand Up @@ -748,17 +766,6 @@ object RefChecks {
checkMemberTypesOK()
checkCaseClassInheritanceInvariant()
}
else if (clazz.is(Trait) && !(clazz derivesFrom defn.AnyValClass))
// For non-AnyVal classes, prevent abstract methods in interfaces that override
// final members in Object; see #4431
for (decl <- clazz.info.decls) {
// Have to use matchingSymbol, not a method involving overridden symbols,
// because the scala type system understands that an abstract method here does not
// override a concrete method in Object. The jvm, however, does not.
val overridden = decl.matchingDecl(defn.ObjectClass, defn.ObjectType)
if (overridden.is(Final))
report.error(TraitRedefinedFinalMethodFromAnyRef(overridden), decl.srcPos)
}

if (!clazz.is(Trait)) {
// check that parameterized base classes and traits are typed in the same way as from the superclass
Expand Down
22 changes: 22 additions & 0 deletions tests/neg/i11130.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
@main def test: Unit = {
class Foo
class Bar extends Foo

trait S[-A] {
type T >: A
}
trait PFoo extends S[Foo]
trait PBar extends S[Bar] {
override type T = Bar
}
class PFooBar extends PBar with PFoo {
override type T >: Bar // error
}

def patmat[A](s: S[A]): s.T = s match {
case p: (PFoo & s.type) => (new Foo): p.T
}

// ClassCastException: Foo cannot be cast to class Bar
val x: Bar = patmat(new PFooBar: PBar)
}
4 changes: 0 additions & 4 deletions tests/neg/i7359-b.check

This file was deleted.

3 changes: 0 additions & 3 deletions tests/neg/i7359-b.scala

This file was deleted.

4 changes: 0 additions & 4 deletions tests/neg/i7359-c.check

This file was deleted.

3 changes: 0 additions & 3 deletions tests/neg/i7359-c.scala

This file was deleted.

4 changes: 0 additions & 4 deletions tests/neg/i7359-d.check

This file was deleted.

3 changes: 0 additions & 3 deletions tests/neg/i7359-d.scala

This file was deleted.

4 changes: 0 additions & 4 deletions tests/neg/i7359-e.check

This file was deleted.

3 changes: 0 additions & 3 deletions tests/neg/i7359-e.scala

This file was deleted.

5 changes: 3 additions & 2 deletions tests/neg/i7359.check
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
-- [E105] Syntax Error: tests/neg/i7359.scala:3:6 ----------------------------------------------------------------------
-- Error: tests/neg/i7359.scala:3:6 ------------------------------------------------------------------------------------
3 | def notify(): Unit // error
| ^
| Traits cannot redefine final method notify from class AnyRef.
| error overriding method notify in class Object of type (): Unit;
| method notify of type (): Unit cannot override final member method notify in class Object
4 changes: 2 additions & 2 deletions tests/neg/inline-override.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ object Test {
override def f1(): Int // OK
override def f2(): Int // OK
override def f3(): Int // error
override def f4(): Int // OK not retained
override def f4(): Int // error
}

abstract class E extends A { // error f1
inline override def f1(): Int
inline override def f2(): Int
inline override def f3(): Int
inline override def f4(): Int // OK not retained
inline override def f4(): Int // error
}

}