From 72054967e28a4b93b61bc010c7f06156372627fa Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 15 Dec 2023 11:43:05 +0100 Subject: [PATCH] Require `@publicInBinary` on overrides --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 5 +---- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 11 +++++++---- library/src/scala/annotation/publicInBinary.scala | 2 +- tests/neg/publicInBinaryOverride.check | 5 +++++ tests/neg/publicInBinaryOverride.scala | 9 +++++++++ tests/run/publicInBinary/Lib_1.scala | 5 +++-- 6 files changed, 26 insertions(+), 11 deletions(-) create mode 100644 tests/neg/publicInBinaryOverride.check create mode 100644 tests/neg/publicInBinaryOverride.scala 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/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 85781c500753..fb692446f1df 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/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 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/neg/publicInBinaryOverride.check b/tests/neg/publicInBinaryOverride.check new file mode 100644 index 000000000000..e44692c78525 --- /dev/null +++ b/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 diff --git a/tests/neg/publicInBinaryOverride.scala b/tests/neg/publicInBinaryOverride.scala new file mode 100644 index 000000000000..4b9144d27540 --- /dev/null +++ b/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 = () 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