bug icon indicating copy to clipboard operation
bug copied to clipboard

Cannot override `protected[Base]` methods

Open Atry opened this issue 4 years ago • 16 comments

reproduction steps

using Scala 2.13.6,

Welcome to Scala 2.13.6 (OpenJDK 64-Bit Server VM, Java 17).
Type in expressions for evaluation. Or try :help.

scala> trait Base {
     |   protected[Base] def p: Unit
     | }
     | 
     | object Base {
     |   class Child extends Base {
     |     protected[Base] def p: Unit = {
     |     }
     |   }
     | }
           protected[Base] def p: Unit = {
                               ^
On line 7: error: weaker access privileges in overriding
       protected[trait Base] def p: Unit (defined in trait Base)
         override should at least be protected[Base]

problem

The error message is confusing because it requires at least protected[Base] while the accessor is indeed protected[Base]

Atry avatar Nov 13 '21 23:11 Atry

~The error message should be error: Base is not an enclosing class~

lrytz avatar Nov 15 '21 09:11 lrytz

@lrytz

The error message should be error: Base is not an enclosing class

Are you sure? protected[Base] should be accessible in both class/trait and companion object Base AFAIK.

Jasper-M avatar Nov 15 '21 09:11 Jasper-M

Hmm.. The spec seems to requrie Base to be an enclosing definition?

A protected modifier can be qualified with an identifier C (e.g. protected[C]) that must denote a class or package enclosing the definition.

https://www.scala-lang.org/files/archive/spec/2.13/05-classes-and-objects.html#protected

Accessing a protected[C] definition would be allowed from C's companion, but I don't see anything about allowing a protected[C] definition in C's companion.

OTOH, the implementation seems to allow protected[C]/private[C] definitions in C's companion, the following compiles with both 2.13 and 3:

trait C {
  //protected[C] def p: Int
}
     
object C {
  class Child extends C {
    protected[C] def p = 1
    private[C] def u = 1
  }
}

Un-commenting the abstract method makes it fail with 2.13 and 3.

lrytz avatar Nov 15 '21 10:11 lrytz

The spec seems to require Base to be an enclosing definition

Base should be an enclosing class or package (or object?). But I always thought that the Base "scope" refers to both the enclosing class or object or package and the potential companion class or object.

This example seems to support my interpretation. The fact that this compiles can't be explained by object Foo having access to private methods of companion class Foo. It can only be explained if the "scope" of Foo in private[Foo] includes both class and object Foo.

class Foo {
  class Bar { private[Foo] def bar = 1 }
}
object Foo {
  val f = new Foo()
  new f.Bar().bar
}

Maybe there's a reason why overriding a protected method follows a different rule, but I don't know what that reason is.

Jasper-M avatar Nov 15 '21 10:11 Jasper-M

Maybe I don't understand it right, but I think your example is fine because the spec explicitly mentions the companion for access:

Members labeled with such a modifier (protected[C]) are accessible ... from code inside the class C and its companion module

The spec doesn't seem to mention the companion for protected/private[C] definitions though, it only says C has to be an enclosing definition.

I assume the implementation allows this intentionally, and that the spec should be fixed. In this case the original issue of this ticket is a bug.

lrytz avatar Nov 15 '21 11:11 lrytz

Ok I see what you mean. The distinction between access and definitions seems kind of arbitrary and oversighty though.

Jasper-M avatar Nov 15 '21 11:11 Jasper-M

FWIW, I don't think it's a bug, but it is a reasonable expectation, and the spec and impl ought to be tweaked.

The other thing you can't do is name enclosing template in case of shadowing, possibly there is a ticket.

package p
class C { protected[C] def f = 42 } 
object C { class C extends p.C { protected[p.C] def f = 42 } }

som-snytt avatar Nov 15 '21 16:11 som-snytt

Ah. Yeah it's not a bug. I got confused by [X] looking like a type argument but it isn't; X is just the next enclosing (type or term) name X.

lrytz avatar Nov 15 '21 20:11 lrytz

I'm confused why it creates two symbols for object Base. How do I know which is which? A quickie check to handle "it's just the companion" doesn't work for that reason, using analyzer.companionSymbolOf for the look-up. At least I know better than to use sym.companionSymbol.

som-snytt avatar Nov 24 '21 06:11 som-snytt

For an object O, there's the "module symbol" O, which is a term symbol and corresponds to the singleton instance at run-time, and the "module class symbol", which is a class symbol (with a parents and a scope for members), that corresponds to the class O$.

scala> object O

scala> val oc = symbolOf[O.type] // ModuleClass

scala> oc.info // ClassInfoType
val res11: $r.intp.global.Type =
AnyRef {
  def <init>(): O.type
}

scala> val o = oc.module // Module

scala> o.info // TypeRef
val res12: $r.intp.global.Type = O.type

lrytz avatar Nov 24 '21 07:11 lrytz

Thanks, that was the rusty api. I confirmed that companionSymbol doesn't work for local companions. Can I do better than ad hoc? as in the linked PR.

I did not tweak the spec, which will probably consume a day with word twisting.

som-snytt avatar Nov 24 '21 18:11 som-snytt

companionSymbol doesn't work for local companions. Can I do better than ad hoc?

I don't think so...

lrytz avatar Nov 30 '21 10:11 lrytz

companionSymbol doesn't work for local companions. Can I do better than ad hoc?

I don't think so...

An idea would be to store the companion of a local class / object as a symbol attachment. It seems this would allow to fix sym.companionClass/companionModule and we could get rid of companionSymbolOf.

lrytz avatar Dec 06 '21 09:12 lrytz

I haven't yet looked at the order of events when typechecking a block vs template. Also haven't tried what dotty would do.

som-snytt avatar Dec 06 '21 16:12 som-snytt

Wouldn't you get the desired result by writing just protected instead of protected[Base] in the trait definition? IIUC, you propose that these two should be equivalent. Which would make the current behaviour of protected[Base] unavailable (unless there's some other way, but I don't see any OTOH). So I'm against changing, it works well and actually adds expressiveness.

ByteEater-pl avatar Aug 07 '22 05:08 ByteEater-pl

@ByteEater-pl the member method could belong to an inner class which is not Base.

This is a syntax problem: it's no longer possible to write protected[Base] at the new definition site.

However, since companions have private access privilege, and protected[X] is a wider restriction, it is allowed.

This is the behavior for Scala 3.

som-snytt avatar Aug 07 '22 11:08 som-snytt