Require @publicInBinary on overrides

Author: nicolasstucki

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -1033,10 +1033,7 @@ object SymDenotations {
 
     /** Is this a member that will become public in the generated binary */
     def hasPublicInBinary(using Context): Boolean =
-      isTerm && (
-        hasAnnotation(defn.PublicInBinaryAnnot) ||
-        allOverriddenSymbols.exists(sym => sym.hasAnnotation(defn.PublicInBinaryAnnot))
-      )
+      isTerm && hasAnnotation(defn.PublicInBinaryAnnot)
 
     /** ()T and => T types should be treated as equivalent for this symbol.
      *  Note: For the moment, we treat Scala-2 compiled symbols as loose matching,

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -298,9 +298,10 @@ object RefChecks {
    *    1.9. If M is erased, O is erased. If O is erased, M is erased or inline.
    *    1.10.  If O is inline (and deferred, otherwise O would be final), M must be inline
    *    1.11.  If O is a Scala-2 macro, M must be a Scala-2 macro.
-   *    1.12.  If O is non-experimental, M must be non-experimental.
-   *    1.13   Under -source future, if O is a val parameter, M must be a val parameter
+   *    1.12.  Under -source future, if O is a val parameter, M must be a val parameter
    *           that passes its value on to O.
+   *    1.13.  If O is non-experimental, M must be non-experimental.
+   *    1.14.  If O has @publicInBinary, M must have @publicInBinary.
    *  2. Check that only abstract classes have deferred members
    *  3. Check that concrete classes do not have deferred definitions
    *     that are not implemented in a subclass.
@@ -571,13 +572,15 @@ object RefChecks {
           overrideError(i"needs to be declared with @targetName(${"\""}${other.targetName}${"\""}) so that external names match")
         else
           overrideError("cannot have a @targetName annotation since external names would be different")
-      else if other.is(ParamAccessor) && !isInheritedAccessor(member, other) then // (1.13)
+      else if other.is(ParamAccessor) && !isInheritedAccessor(member, other) then // (1.12)
         report.errorOrMigrationWarning(
             em"cannot override val parameter ${other.showLocated}",
             member.srcPos,
             MigrationVersion.OverrideValParameter)
-      else if !other.isExperimental && member.hasAnnotation(defn.ExperimentalAnnot) then // (1.12)
+      else if !other.isExperimental && member.hasAnnotation(defn.ExperimentalAnnot) then // (1.13)
         overrideError("may not override non-experimental member")
+      else if !member.hasAnnotation(defn.PublicInBinaryAnnot) && other.hasAnnotation(defn.PublicInBinaryAnnot) then // (1.14)
+        overrideError("also needs to be declared with @publicInBinary")
       else if other.hasAnnotation(defn.DeprecatedOverridingAnnot) then
         overrideDeprecation("", member, other, "removed or renamed")
     end checkOverride

library/src/scala/annotation/publicInBinary.scala

@@ -1,6 +1,6 @@
 package scala.annotation
 
-/** A binary API is a definition that is annotated with `@publicInBinary` or overrides a definition annotated with `@publicInBinary`.
+/** A binary API is a definition that is annotated with `@publicInBinary`.
  *  This annotation can be placed on `def`, `val`, `lazy val`, `var`, class constructors, `object`, and `given` definitions.
  *  A binary API will be publicly available in the bytecode. Tools like TASTy MiMa will take this into account to check
  *  compatibility.

tests/neg/publicInBinaryOverride.check

@@ -0,0 +1,5 @@
+-- [E164] Declaration Error: tests/neg/publicInBinaryOverride.scala:8:15 -----------------------------------------------
+8 |  override def f(): Unit = () // error
+  |               ^
+  |               error overriding method f in class A of type (): Unit;
+  |                 method f of type (): Unit also needs to be declared with @publicInBinary

tests/neg/publicInBinaryOverride.scala

@@ -0,0 +1,9 @@
+import scala.annotation.publicInBinary
+
+class A:
+  @publicInBinary def f(): Unit = ()
+  @publicInBinary def g(): Unit = ()
+
+class B extends A:
+  override def f(): Unit = () // error
+  @publicInBinary override def g(): Unit = ()

tests/run/publicInBinary/Lib_1.scala

@@ -21,14 +21,15 @@ class Foo(@publicInBinary private[Foo] val paramVal: Int, @publicInBinary privat
     paramVal + paramVar + protectedVal + packagePrivateVal + protectedVar + packagePrivateVar
 
 class Bar() extends Foo(3, 3):
+  @publicInBinary
   override protected val protectedVal: Int = 2
-
+  @publicInBinary
   override private[foo] val packagePrivateVal: Int = 2
 
   inline def bar: Int = protectedVal + packagePrivateVal
 
 class Baz() extends Foo(4, 4):
-  @publicInBinary // TODO warn? Not needed because Foo.protectedVal is already @publicInBinary
+  @publicInBinary
   override protected val protectedVal: Int = 2
 
   @publicInBinary