From 9789d51ccf0bfa264204d4bf1c8b61ef2ed4f5fe Mon Sep 17 00:00:00 2001
From: Nicolas Stucki <nicolas.stucki@gmail.com>
Date: Fri, 15 Dec 2023 11:43:05 +0100
Subject: [PATCH] Require `@publicInBinary` on overrides

---
 compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 5 +----
 compiler/src/dotty/tools/dotc/typer/Checking.scala      | 6 ++++++
 library/src/scala/annotation/publicInBinary.scala       | 2 +-
 tests/run/publicInBinary/Lib_1.scala                    | 5 +++--
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
index 902bbda8bead..b1e85f2b4f90 100644
--- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/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,
diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala
index 03b9bf0a91db..a118f0541d79 100644
--- a/compiler/src/dotty/tools/dotc/typer/Checking.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala
@@ -544,6 +544,12 @@ object Checking {
       else if !sym.owner.isClass && !(sym.is(Param) && sym.owner.isConstructor) then fail(em"@publicInBinary cannot be used on local definitions")
       else if sym.is(ParamAccessor) && sym.is(Private) then fail(em"@publicInBinary cannot be non `val` constructor parameters")
       else if sym.is(Private) && !sym.privateWithin.exists && !sym.isConstructor then fail(em"@publicInBinary cannot be used on private definitions\n\nConsider using `private[${sym.owner.name}]` or `protected` instead")
+    else
+      sym.allOverriddenSymbols.find(sym => sym.hasAnnotation(defn.PublicInBinaryAnnot)) match
+        case Some(overriddenSym) =>
+          fail(em"""This definitions ${if sym.is(Override) then "overrides" else "implements"} @publicInBinary ${overriddenSym.showLocated}.
+                   |This definition must also be marked as @publicInBinary.""")
+        case None => ()
     if (sym.hasAnnotation(defn.NativeAnnot)) {
       if (!sym.is(Deferred))
         fail(NativeMembersMayNotHaveImplementation(sym))
diff --git a/library/src/scala/annotation/publicInBinary.scala b/library/src/scala/annotation/publicInBinary.scala
index a13e00c08c12..4990d266f892 100644
--- a/library/src/scala/annotation/publicInBinary.scala
+++ b/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.
diff --git a/tests/run/publicInBinary/Lib_1.scala b/tests/run/publicInBinary/Lib_1.scala
index 53cc7ee16c33..a3c6ccea8427 100644
--- a/tests/run/publicInBinary/Lib_1.scala
+++ b/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