Skip to content

Override check does not account for match type. #7894

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
esse-byte opened this issue Jan 5, 2020 · 4 comments · Fixed by #12153
Closed

Override check does not account for match type. #7894

esse-byte opened this issue Jan 5, 2020 · 4 comments · Fixed by #12153
Assignees
Milestone

Comments

@esse-byte
Copy link

minimized code

scala> case class Box[T](t: T)
// defined case class Box

scala> type Boxed[T <: Tuple] <: Tuple = T match {
     |   case Unit => Unit
     |   case h *: t => Box[h] *: Boxed[t]
     | }

scala> trait Cmp[T <: Tuple] { def cmp(t: T, b: Boxed[T]): Boolean }
// defined trait Cmp

scala> object UnitCmp extends Cmp[Unit] {
     |   def cmp(t: Unit, b: Unit): Boolean = true
     | }
2 |  def cmp(t: Unit, b: Unit): Boolean = true
  |      ^
  |error overriding method cmp in trait Cmp of type (t: Unit, b: Boxed[Unit]): Boolean;
  |  method cmp of type (t: Unit, b: Unit): Boolean has incompatible type
1 |object UnitCmp extends Cmp[Unit] {
  |       ^
  |object creation impossible, since def cmp(t: Unit, b: Boxed[Unit]): Boolean is not defined
  |(The class implements a member with a different type: def cmp(t: Unit, b: Unit): Boolean)

scala> object UnitCmp extends Cmp[Unit] {
     |   def cmp(t: Unit, b: Boxed[Unit]): Boolean = true
     | }
2 |  def cmp(t: Unit, b: Boxed[Unit]): Boolean = true
  |      ^
  |error overriding method cmp in trait Cmp of type (t: Unit, b: Boxed[Unit]): Boolean;
  |  method cmp of type (t: Unit, b: Unit): Boolean has incompatible type
1 |object UnitCmp extends Cmp[Unit] {
  |       ^
  |object creation impossible, since def cmp(t: Unit, b: Boxed[Unit]): Boolean is not defined
  |(The class implements a member with a different type: def cmp(t: Unit, b: Unit): Boolean)

expectation

I'm not sure if it is just the limitation of Match Type or a bug.

@esse-byte esse-byte changed the title The Match type cannot be recognize in overrided method. The Match type cannot be recognized in overrided method. Jan 5, 2020
@esse-byte esse-byte changed the title The Match type cannot be recognized in overrided method. Override check does not account for match type. Jan 5, 2020
@odersky
Copy link
Contributor

odersky commented Jan 6, 2020

This is as intended but needs to be specced. We need to have a clause that a method A overrides a method B only if both the original types and the erased types conform (or, alternatively, original types and signatures, if we want to talk about that). The erasure of a match type is the erasure of its upper bound, which would be Any in the example.

@odersky odersky self-assigned this Jan 6, 2020
@sjrd
Copy link
Member

sjrd commented Jan 6, 2020

Hum, aren't bridges supposed to relieve us from the necessity that erased types have to conform? Today we can override a

def foo(x: T): Int

with a

def foo(x: Int): Int

when the class' type parameter is instantiated to Int in a subclass.

@Blaisorblade
Copy link
Contributor

@odersky > We need to have a clause that a method A overrides a method B only if both the original types and the erased types conform (or, alternatively, original types and signatures, if we want to talk about that).

That's unexpected; if correct, IIUC, that might affect #7597 as well?

@odersky
Copy link
Contributor

odersky commented Feb 19, 2020

It's the signatures as seen from the subclass that matter here, not the erased types per se. Bridges
help us bridge the as-seen-from. If we would base overriding checks on full types, two things would happen:

  • member resolution would become computationally a lot more expensive
  • we'd lose the ability to refer to members of other compilation units via their signatures

Together, this would mean we'd have to start from scratch and write another compiler.

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

Successfully merging a pull request may close this issue.

5 participants