Skip to content

@publicInBinary override spec change #19296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion library/src/scala/annotation/publicInBinary.scala
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
5 changes: 5 additions & 0 deletions tests/neg/publicInBinaryOverride.check
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions tests/neg/publicInBinaryOverride.scala
Original file line number Diff line number Diff line change
@@ -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 = ()
5 changes: 3 additions & 2 deletions tests/run/publicInBinary/Lib_1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down