Scala Programming Language
  1. Scala Programming Language
  2. SI-2080

It should not be possible to override concrete member types

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Misc Compiler
    • Labels:

      Description

      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.

        Issue Links

          Activity

          Vladimir Reshetnikov created issue -
          Hide
          Mark Harrah added a comment -

          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
          
          Show
          Mark Harrah added a comment - 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
          Hide
          Martin Odersky added a comment -

          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.

          Show
          Martin Odersky added a comment - 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.
          Hide
          Mark Harrah added a comment -

          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?

          Show
          Mark Harrah added a comment - 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?
          Hide
          Simon Ochsenreither added a comment -

          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?

          Show
          Simon Ochsenreither added a comment - 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?
          Lukas Rytz made changes -
          Field Original Value New Value
          Workflow jira [ 21289 ] scala [ 24358 ]
          Yuvi Masory made changes -
          Workflow scala [ 24358 ] Open-Closed Workflow v3 [ 29822 ]
          Hide
          Simon Ochsenreither added a comment -
          Show
          Simon Ochsenreither added a comment - Pull request for test: https://github.com/scala/scala/pull/18
          Hide
          Paul Phillips added a comment -

          See SI-7278 for some illustrations of the unsoundness being perpetrated here.

          Show
          Paul Phillips added a comment - See SI-7278 for some illustrations of the unsoundness being perpetrated here.
          Paul Phillips made changes -
          Labels soundness
          Simon Ochsenreither made changes -
          Link This issue relates to SI-7278 [ SI-7278 ]

            People

            • Assignee:
              Martin Odersky
              Reporter:
              Vladimir Reshetnikov
              TracCC:
              arjan blokzijl, Christos KK Loverdos, Ismael Juma, Mark Harrah, Paul Phillips, Seth Tisue, Simon Ochsenreither
            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development