Skip to content

Commit

Permalink
@publicInBinary override spec change (#19296)
Browse files Browse the repository at this point in the history
Followup of #18402 (see
#18402 (comment)).

This was split from the main PR due to the blocker described in
#18402 (comment).
During the Dotty meeting, we decided to merge #18402 without the second
commit ([Require @publicInBinary on
overrides](34f3dee)).
If we manage to fix this issue before 3.4.0-RC1 we will include it
there, otherwise it will be part of 3.4.0-RC2.
  • Loading branch information
nicolasstucki authored Dec 19, 2023
2 parents 8d9da73 + 7205496 commit eae8831
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 11 deletions.
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

0 comments on commit eae8831

Please sign in to comment.