Skip to content
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

A method implementing an abstract method is not a valid override for a repeated declaration of the same method in a third class. #12946

Closed
noresttherein opened this issue Feb 11, 2024 · 7 comments

Comments

@noresttherein
Copy link

noresttherein commented Feb 11, 2024

Reproduction steps

Scala version: 2.13.12

trait Base {
	def m() :Unit
}

class Impl extends Base {
	override def m() :Unit = ()
}

abstract class Spec extends Impl {
	override def m() :Unit
}

class SpecImpl extends Spec

Problem

class SpecImpl needs to be abstract.
No implementation found in a subclass for deferred declaration
override def m(): Unit (defined in class Spec)
class SpecImpl extends Spec

In my reading of the overriding section of SLS, this should work, but I am prepared to be said otherwise.
While one could easily simply remove the repeated declaration, it is often useful to change scaladocs or annotations (including @throws) wthout implementing a method. Considering how annoying the 'concrete implementation always overrides an abstract declaration' rule can be, it would be the one place where it would be actually useful.

@SethTisue
Copy link
Member

Scala 3 rejects it as well. I strongly suspect this is working as designed. The solution is to call super.m() when overriding.

Java rejects it, too:

public interface Base {
  abstract void m();
}

class Impl implements Base {
  public void m() { }
}

abstract class Spec extends Impl {
  public abstract void m();
}

class SpecImpl extends Spec { }
Base.java:13: error: SpecImpl is not abstract and does not override abstract method m() in Spec
class SpecImpl extends Spec { }
^
1 error

In my reading of the overriding section of SLS, this should work,

What particular passage(s) do you have in mind?

@som-snytt
Copy link

I'm up for my annual certification class, so I looked briefly before bookmarking.

The section is

Time to invite your friends and let them know that Bluesky is making social fun again!

oops wrong paste

https://scala-lang.org/files/archive/spec/2.13/05-classes-and-objects.html#class-members

where it says an abstract member is the last one in linearization order, unless the class "contains already" a concrete member.

Then the "two rules" are restated in terms of overriding.

@lrytz
Copy link
Member

lrytz commented Feb 12, 2024

Related #12892

@lrytz
Copy link
Member

lrytz commented Feb 12, 2024

Ah, it's not the "who overrides whom" check that's wrong, but it's the "are there any abstract methods" check.

class A {
	def m(): Unit = ()
}
abstract class B extends A {
	def m(): Unit
}
abstract class C extends B

This compiles. Remove abstract from C and the compiler prints class C needs to be abstract.

@sjrd
Copy link
Member

sjrd commented Feb 12, 2024

I have a vague recollection that this used to be allowed, but we intentionally disallowed a few years ago for some reason. Perhaps there was some issue with bytecode generation, and since Java did not allow it, it was the easiest path forward? Maybe I'm also remembering it all wrong.

@noresttherein
Copy link
Author

I am not saying it should be accepted, just that the runtime error is amusing. The report stemmed from the fact that I extended a @specialized trait thinking it was not specialized, and thus the 'override m() :Intwas needed so that clients ofImpl` (which originally was a trait in diamond inheritance) have access to a non-boxing method.

I quoted the rule from memory, so I might not remember the exact wording, but I am painfully aware of it, because it causes issues in 'Ops' pattern:

trait Ops[C] {
  def m1 :C
  def m2(other :Impl with Ops[C]) :C = other.m1
}
trait Impl extends Ops[Impl] {
  override def m1 :Impl = this
}

Does not compile. And I am also allowed to override final methods this way:

trait MyColl[T, CC[_], C] extends IterableOps[T, CC, C] {
	def ++[U >: T](other :IterableOnce[U]) :CC[U] //++ is final in IterableOps
}
class MyCollImpl[T] extends Iterable[T] with MyColl[T, MyCollImpl, MyCollImpl[T]] with IterableFactoryDefaults[T, MyCollImpl] {
	override def iterator :Iterator[T] = ???
	override def iterableFactory :IterableFactory[MyCollImpl] = ???
}

This compiles and is useful when you want to provide more specific docs. So the expectation that the code in the reported bug should work isn't far fetched.

@noresttherein
Copy link
Author

Also, very tangentially connected: there are plenty cases in scala.collection (maybe the majority) where an implementing method doesn't use override keyword. This is problematic, because it then can't override another implementation of the same method mixed in before it in linearization order. From memory,

trait IndexedBuffer[A] extends Buffer[A] {
  def knownSize = length
}
class MyBuffer[A] extends Buffer[A] { 
  override def knownSize = length
  ...
}
new MyBuffer[A] with IndexedBuffer[A] // <- won't compile because IndexedBuffer.knownSize doesn't have the override keyword, despite both implementing the exact same method..

This is pseudo code, but I hope you see what I'm talking about.

@SethTisue SethTisue closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants