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

It should not be possible to override concrete member types #2080

Closed
scabug opened this issue Jun 19, 2009 · 8 comments
Closed

It should not be possible to override concrete member types #2080

scabug opened this issue Jun 19, 2009 · 8 comments
Assignees
Labels
fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) should not compile
Milestone

Comments

@scabug
Copy link

scabug commented Jun 19, 2009

trait A {
 type T
 def f(x : T) : T
}

trait B extends A {
 trait T { }
 override def f(x : T) : T = x
}

object C extends B {
 override trait T {
   def g = {  }
 }
 override def f(x : T) : T = { x.g; x }
}

It compiles without errors, but T in B and T in C are completely unrelated types.

@scabug
Copy link
Author

scabug commented Jun 19, 2009

Imported From: https://issues.scala-lang.org/browse/SI-2080?orig=1
Reporter: Vladimir Reshetnikov (nikov)
See #7278

@scabug
Copy link
Author

scabug commented Jun 5, 2010

@harrah said:

T in C overrides the abstract type in A and f in C overloads the method f in A. Both 'override' modifiers are optional here.
The T traits in B and C are indeed different types to the compiler. You can see this with classes as well (example at the end). Additionally, you can make them related by having the trait in C extend super.T.

Details:

Line 387 in !RefChecks explicitly skips checking overridden classes:

  if (!opc.overridden.isClass) checkOverride(clazz, opc.overriding, opc.overridden);

This particular line is 5 years old (r4511) and the behavior existed in the code it replaced. It appears to have been this way since !RefChecks existed (r4246).

If I remove the !isClass, I get the error probably expected by the reporter:

error: overriding trait T in trait B;
 trait T cannot be used here - classes and objects cannot be overridden
        override trait T {
                       ^

or for the example at the end:

Q.scala:3: error: overriding class Q in class M;
 class Q cannot be used here - classes and objects cannot be overridden
class N extends M { class Q { def x = 5 } }
                          ^

However, I think the current behavior might be intended. Compiling the standard library with the check removed shows that the collections library has things like:

  trait Transformed[+B] extends IterableView[B, Coll] with super.Transformed[B]

and Numeric shadows Ops in Ordering:

src/library/scala/math/Numeric.scala:189: error: overriding class Ops in trait Ordering;
class Ops cannot be used here - classes and objects cannot be overridden
class Ops(lhs: T) {
      ^

Since this appears to be intended, I would guess that this bug is invalid.
I've read the relevant definitions (matches, subsumes, overrides) in Ch. 2, 3, and 5, and I'm not sure what exactly allows this in the spec.
Perhaps closing this bug would just be a matter of updating or clarifying the spec?

While the spec update can probably wait, perhaps it is an immediate issue to just verify that the behavior is indeed expected. Otherwise, the collections library makes it to final depending on a bug.

Example showing different classes and methods:

scala> class M { class Q { def x = 3 }; def q(a: Q) = a.x }
defined class M

scala> class N extends M { class Q { def x = 5 }; def q(a: Q) = a.x }
defined class N

scala> val m = new M
m: M = M@10aefdb

scala> val n = new N
n: N = N@1ce08c7

scala> val nm: M = n
nm: M = N@1ce08c7

scala> val mq = new m.Q
mq: m.Q = M$$Q@191f61c

scala> val nq = new n.Q
nq: n.Q = N$$Q@938b4a

scala> val nmq = new nm.Q
nmq: nm.Q = M$$Q@1ab7626

scala> m.q(mq)
res0: Int = 3

scala> m.q(nq)
<console>:11: error: type mismatch;
 found   : n.Q
 required: m.Q
       m.q(nq)
           ^

scala> m.q(nmq)
<console>:12: error: type mismatch;
 found   : nm.Q
 required: m.Q
       m.q(nmq)
           ^

scala> nm.q(nmq)
res3: Int = 3

scala> n.q(nmq)
<console>:11: error: overloaded method value q with alternatives:
  (a: n.Q(in class N))Int <and>
  (a: n.Q(in class M))Int
 cannot be applied to (nm.Q(in class M))
       n.q(nmq)
         ^

scala> (n: M).q(nmq)
<console>:11: error: type mismatch;
 found   : nm.Q
 required: _1.Q where val _1: M
       (n: M).q(nmq)
                ^

scala> n.q(nq)
res6: Int = 5

@scabug
Copy link
Author

scabug commented Jun 5, 2010

@odersky said:
The behavior in views is as expected. The ticket is still correct in that override makes no sense here. In fact, the spec says that overrid9ing simply cannot apply for traits or classes. This is one thing that might be revised in future versions of Scala. So, with that in mind, we still need to update the spec. But it's not super urgent to do so.

Thanks, Mark, for raising the point, though.

@scabug
Copy link
Author

scabug commented Jun 5, 2010

@harrah said:
Thanks, Martin. No rush on the following:

In the spec, under Overriding it says:
M matches M' implies M overrides M' and M must subsume M'.

(Hopefully I'm not paraphrasing away important distinctions here.)

I get from the spec that a trait/class in a subclass matches a trait/class in the superclass with the same name because neither is a method declaration. I assume 'bind the same name' includes the type/value namespace distinction so that classes don't match vals and vice versa.

I saw that a trait/class doesn't subsume another trait/class, so overriding can't happen. I assumed it would be an error if something matches but doesn't subsume. If this is correct, it is the definition of matches that needs to be updated to exclude classes/traits?

Is the actual change to the compiler just to disallow the 'override' modifier on nested traits/classes?

@scabug
Copy link
Author

scabug commented Mar 4, 2011

@soc said:
The original example does not compile anymore on 2.9.0.r24324-b20110222020042:

scala> object C extends B {
     |  override trait T {
     |    def g {  }
     |  }
     |  override def f(x : T) : T = { x.g; x }
     | }
<console>:13: error: illegal combination of modifiers: abstract and override for: trait T
        override trait T {
                       ^

Is this fixed?

@scabug
Copy link
Author

scabug commented Dec 2, 2011

@soc said:
Pull request for test: scala/scala#18

@scabug
Copy link
Author

scabug commented Dec 19, 2013

@paulp said:
See #7278 for some illustrations of the unsoundness being perpetrated here.

@som-snytt
Copy link

som-snytt commented Jul 7, 2020

Deprecation (and error under -Xsource:3) at scala/scala#8705

@SethTisue SethTisue modified the milestones: Backlog, 2.13.2 Jul 29, 2020
@SethTisue SethTisue added the fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in Scala 3 This issue does not exist in the Scala 3 compiler (https://github.com/lampepfl/dotty/) should not compile
Projects
None yet
Development

No branches or pull requests

5 participants