Skip to content

Unsoundness due to invalid inheritance being permitted #5817

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

Closed
abgruszecki opened this issue Jan 29, 2019 · 6 comments
Closed

Unsoundness due to invalid inheritance being permitted #5817

abgruszecki opened this issue Jan 29, 2019 · 6 comments
Assignees

Comments

@abgruszecki
Copy link
Contributor

abgruszecki commented Jan 29, 2019

object Test {
  trait A[+X] { def x: X }
  class B[+X](val x: X) extends A[X]
  class C[+X](x: Any) extends B[Any](x) with A[X]

  def get(a: A[Int]): Int = a.x
  get(new C[Int]("foo")) // class cast exception: String is not Int
}

Note that we previously came to the conclusion that we do not want to prevent inheritance like C in general: #3989.

The likely fix would be to require C to override B#x - this is precisely what Scalac does.

@abgruszecki
Copy link
Contributor Author

@smarter quick question - is this refchecks?

@smarter
Copy link
Member

smarter commented Jan 29, 2019

The fix for #3989 was done in refchecks so it would make sense to do the same here I guess.

@odersky
Copy link
Contributor

odersky commented Jan 29, 2019

Rename the x to y in C and it stops compiling:

object Test {
  trait A[+X] { def x: X }
  class B[+X](val x: X) extends A[X]
  class C[+X](y: Any) extends B[Any](y) with A[X]

  def get(a: A[Int]): Int = a.x
  get(new C[Int]("foo")) // class cast exception: String is not Int
}

You get:

4 |  class C[+X](y: Any) extends B[Any](y) with A[X]
  |        ^
  |        value x in class B is not a legal implementation of `x' in class C
  |          its type             Any
  |          does not conform to  => X

There's a special code path in PostTyper that merges the values of x in B and C in order to avoid creating unnecessary field slots. That logic seems to be missing a test.

@smarter
Copy link
Member

smarter commented Jan 29, 2019

There's a special code path in PostTyper that merges the values of x in B and C in order to avoid creating unnecessary field slots.

Link for anyone looking for it: https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/transform/ParamForwarding.scala

@odersky
Copy link
Contributor

odersky commented Feb 1, 2019

I got as far as avoiding the param forwarder, but the problem persists. So this goes deeper.

@odersky odersky self-assigned this Feb 1, 2019
@dwijnand
Copy link
Member

Looks like this is fixed in at least 3.0.1:

$ scalac -3 i5817.scala
https://repo1.maven.org/maven2/io/get-coursier/apps/maven-metadata.xml
  No new update since 2021-09-02 12:27:05
-- [E163] Declaration Error: i5817.scala:4:8 -----------------------------------
4 |  class C[+X](x: Any) extends B[Any](x) with A[X]
  |        ^
  |        [error overriding method ](url)x in trait A of type => X;
  |          value x in class B of type Any has incompatible type

longer explanation available when compiling with `-explain`
1 error found

Maybe with #11133?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants