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

Allowing contravariant type parameter has inconsistent values breaks the dynamic binding #22373

Open
XYZboom opened this issue Jan 15, 2025 · 3 comments
Labels
area:variance Issues related to covariance & contravariance. itype:bug

Comments

@XYZboom
Copy link

XYZboom commented Jan 15, 2025

Compiler version

3.6.3-RC2, 3.6.4-RC1-bin-20241231-1f0c576-NIGHTLY

Minimized code

open class A
class SubA extends A
class SubA1 extends A
trait B[-E <: A] {
  def func(e: E): Unit
}
trait SubB extends B[SubA] {
  override def func(e: SubA): Unit = println("SubB SubA")
}

trait Base extends B[A] {
  override def func(e: A): Unit = {
    println("Base A")
  }
}

class SubBase extends Base, SubB {
  override def func(e: SubA): Unit = {
    println("SubBase SubA")
  }
}

class B1 extends SubB

def printIt(b: SubB): Unit = {
  b.func(SubA())
  val b1: B[SubA] = b
  b1.func(SubA())
}

object Main {
  def main(args: Array[String]): Unit = {
    printIt(SubBase())
    printIt(B1())
  }
}

Output

SubBase SubA
Base A
SubB SubA
SubB SubA

Expectation

SubBase SubA
SubBase SubA
SubB SubA
SubB SubA

Since b1 and b are the same object and call functions with the same signature, their behavior should be the same.

Reason why this bug occurs

As far as we know, JVM will use bridging methods to call functions in subclasses after type erasure. When calling a method in something such as B[SubA], the actually called method is this bridge method.
The following is the result of using the javap -c -s -v B1 to view the bridging method of Class B1:

public void func(A);
    descriptor: (LA;)V
    flags: ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: aload_1
         2: checkcast     #24                 // class SubA
         5: invokevirtual #26                 // Method func:(LSubA;)V
         8: return

We can see a ACC_BRIDGE, ACC_SYNTHETIC in this method. Actually, this method overrides def func(e: E): Unit in trait B. That's why B1 performed AS EXPECTED.
But in class SubBase the bridge method calls the method in Base:

public void func(A);
    descriptor: (LA;)V
    flags: ACC_PUBLIC, ACC_BRIDGE, ACC_SYNTHETIC
    Code:
      stack=2, locals=2, args_size=2
         0: aload_0
         1: aload_1
         2: invokestatic  #22                 // InterfaceMethod Base.func$:(LBase;LA;)V
         5: return

Since the bridge method in the JVM must exist, we cannot allow SubBase to have inconsistent values. Although semantically feasible.

@XYZboom XYZboom added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 15, 2025
@Gedochao Gedochao added area:variance Issues related to covariance & contravariance. and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 16, 2025
@robstoll
Copy link
Contributor

just as notice, same behaviour in scala 2.13.16

@robstoll
Copy link
Contributor

@XYZboom I am not yet entirely convinced that we should not allow it but maybe should emit a warning.

Maybe not that intuitive but SubBase has two func overloads, one inherited by Base and another by SubB.
In contrast to

class SubBase2 extends Base, B[SubA]

It only inherits the method from Base and due to B[A] <: B[SubA] there is also no need to implement something.
And if you tried:

class SubBase2 extends Base, B[SubA] {
  override def func(e: SubA): Unit = println("SubBase A")
}

it would result in an error with method func has a different signature than the overridden declaration

But back to SubBase, since we have two overloads and I guess they resolve to different upper bounds after type erasure, we kind of get the following after type erasure:

class SubBase {
  def func(e: A)
  def func(e: SubA)
}

if B were defined as def func(e: List[E]): Unit then we would have been out of luck already as they clash after type eraser and scala also emits an error in this case:

Name clash between inherited members:
override def func(e: List[A]): Unit in trait Base at line 12 and
override def func(e: List[SubA]): Unit in trait SubB at line 8
have the same type after erasure.

But again, back to SubBase, you could have overriden fun(e: A) too, to get to the desired behavior. In this sense, your implementation broke the Liskov Substitution Principle as it overrode the function defined in the supertype B[SubA] instead of B[A]and following this reasoning, not scala but the implementation is to blame, no? 😉

Put differently, if you define SubBase as follows:

class SubBase extends Base with SubB {
  override def func(e: A): Unit = println("SubBase A")
  override def func(e: SubA): Unit = func(e:A)
}

then you would see twice the same output as your implementation follows LSP (but I agree, it can ask for trouble, but it is a handy feature sometimes, so please don't take it away from me 👼).

@XYZboom
Copy link
Author

XYZboom commented Jan 17, 2025

@robstoll I agree to use 'warning' instead of prohibiting users from doing so. But I think this is still not safe enough, perhaps it should be prohibited and a compiler option should be set to allow users to turn on this feature, just like some checks in typescript.

Besides, I have another solution. The cause of this bug is that user defined func(e: A): Unit unexpectedly overrides the bridge method func(e: A): Unit = func(e.asInstanceOf[SubA]) maybe we can insert func(e.asInstanceOf[SubA]) before user defined codes. Like this:

// User defined SubBase
class SubBase extends Base, SubB {
  override def func(e: SubA): Unit = {
    println("SubBase SubA")
  }
}
// The actually SubBase generated by compiler
class SubBase extends Base, SubB {
  // ACC_BRIDGE ACC_SYNTHETIC
  override def func(e: A): Unit = {
    if (e.isInstanceOf[SubA]) {
      func(e.asInstanceOf[SubA])
      return
    }
    // if user overrides this function in SubBase,
    // replace super call with user defined codes.
    // if super function is abstract, the above if an this super call is no need.
    return super.func(e) 
  }
  override def func(e: SubA): Unit = {
    println("SubBase SubA")
  }
}

But this solution will loop endlessly on your last example. And does not work when def func(e: A): Unit is final

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:variance Issues related to covariance & contravariance. itype:bug
Projects
None yet
Development

No branches or pull requests

3 participants