Add @binaryapi

Part of the fix for scala#15413
Part of the fix for scala#16983

compiler/src/dotty/tools/dotc/Compiler.scala

@@ -89,7 +89,7 @@ class Compiler {
          new ExplicitOuter,          // Add accessors to outer classes from nested ones.
          new ExplicitSelf,           // Make references to non-trivial self types explicit as casts
          new StringInterpolatorOpt,  // Optimizes raw and s and f string interpolators by rewriting them to string concatenations or formats
-         new DropBreaks) ::          // Optimize local Break throws by rewriting them 
+         new DropBreaks) ::          // Optimize local Break throws by rewriting them
     List(new PruneErasedDefs,        // Drop erased definitions from scopes and simplify erased expressions
          new UninitializedDefs,      // Replaces `compiletime.uninitialized` by `_`
          new InlinePatterns,         // Remove placeholders of inlined patterns
@@ -141,6 +141,7 @@ class Compiler {
          new sjs.JUnitBootstrappers, // Generate JUnit-specific bootstrapper classes for Scala.js (not enabled by default)
          new CollectEntryPoints,     // Collect all entry points and save them in the context
          new CollectSuperCalls,      // Find classes that are called with super
+         new BinaryAPIAnnotations,   // Makes @binaryAPI definitions public // TODO where should this phase be located?
          new RepeatableAnnotations) :: // Aggregate repeatable annotations
     Nil
 

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

@@ -1047,6 +1047,7 @@ class Definitions {
   @tu lazy val RequiresCapabilityAnnot: ClassSymbol = requiredClass("scala.annotation.internal.requiresCapability")
   @tu lazy val RetainsAnnot: ClassSymbol = requiredClass("scala.annotation.retains")
   @tu lazy val RetainsByNameAnnot: ClassSymbol = requiredClass("scala.annotation.retainsByName")
+  @tu lazy val BinaryAPIAnnot: ClassSymbol = requiredClass("scala.annotation.binaryAPI")
 
   @tu lazy val JavaRepeatableAnnot: ClassSymbol = requiredClass("java.lang.annotation.Repeatable")
 

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

@@ -1035,6 +1035,13 @@ object SymDenotations {
       isOneOf(EffectivelyErased)
       || is(Inline) && !isRetainedInline && !hasAnnotation(defn.ScalaStaticAnnot)
 
+    /** Is this a member that will become public in the generated binary */
+    def isBinaryAPI(using Context): Boolean =
+      isTerm && (
+        hasAnnotation(defn.BinaryAPIAnnot) ||
+        allOverriddenSymbols.exists(_.hasAnnotation(defn.BinaryAPIAnnot))
+      )
+
     /** ()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,
      *  because the Scala library does not always follow the right conventions.

compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala

@@ -53,6 +53,9 @@ object PrepareInlineable {
     /** A tree map which inserts accessors for non-public term members accessed from inlined code.
      */
     abstract class MakeInlineableMap(val inlineSym: Symbol) extends TreeMap with Insert {
+
+      def useBinaryAPI: Boolean
+
       def accessorNameOf(name: TermName, site: Symbol)(using Context): TermName =
         val accName = InlineAccessorName(name)
         if site.isExtensibleClass then accName.expandedName(site) else accName
@@ -70,6 +73,7 @@ object PrepareInlineable {
       def needsAccessor(sym: Symbol)(using Context): Boolean =
         sym.isTerm &&
         (sym.isOneOf(AccessFlags) || sym.privateWithin.exists) &&
+        (!useBinaryAPI || !sym.isBinaryAPI) &&
         !sym.isContainedIn(inlineSym) &&
         !(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
         !sym.isInlineMethod &&
@@ -100,7 +104,7 @@ object PrepareInlineable {
      *  possible if the receiver is essentially this or an outer this, which is indicated
      *  by the test that we can find a host for the accessor.
      */
-    class MakeInlineableDirect(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
+    class MakeInlineableDirect(inlineSym: Symbol, val useBinaryAPI: Boolean) extends MakeInlineableMap(inlineSym) {
       def preTransform(tree: Tree)(using Context): Tree = tree match {
         case tree: RefTree if needsAccessor(tree.symbol) =>
           if (tree.symbol.isConstructor) {
@@ -124,13 +128,14 @@ object PrepareInlineable {
      *      private[inlines] def next[U](y: U): (T, U) = (x, y)
      *    }
      *    class TestPassing {
-     *      inline def foo[A](x: A): (A, Int) = {
-     *      val c = new C[A](x)
-     *      c.next(1)
-     *    }
-     *    inline def bar[A](x: A): (A, String) = {
-     *      val c = new C[A](x)
-     *      c.next("")
+     *        inline def foo[A](x: A): (A, Int) = {
+     *        val c = new C[A](x)
+     *        c.next(1)
+     *      }
+     *      inline def bar[A](x: A): (A, String) = {
+     *        val c = new C[A](x)
+     *        c.next("")
+     *      }
      *    }
      *
      *  `C` could be compiled separately, so we cannot place the inline accessor in it.
@@ -143,7 +148,7 @@ object PrepareInlineable {
      *  Since different calls might have different receiver types, we need to generate one
      *  such accessor per call, so they need to have unique names.
      */
-    class MakeInlineablePassing(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
+    class MakeInlineablePassing(inlineSym: Symbol, val useBinaryAPI: Boolean) extends MakeInlineableMap(inlineSym) {
 
       def preTransform(tree: Tree)(using Context): Tree = tree match {
         case _: Apply | _: TypeApply | _: RefTree
@@ -153,7 +158,7 @@ object PrepareInlineable {
           val qual = qualifier(refPart)
           inlining.println(i"adding receiver passing inline accessor for $tree/$refPart -> (${qual.tpe}, $refPart: ${refPart.getClass}, $argss%, %")
 
-          // Need to dealias in order to cagtch all possible references to abstracted over types in
+          // Need to dealias in order to catch all possible references to abstracted over types in
           // substitutions
           val dealiasMap = new TypeMap {
             def apply(t: Type) = mapOver(t.dealias)
@@ -226,8 +231,13 @@ object PrepareInlineable {
         // so no accessors are needed for them.
         tree
       else
-        new MakeInlineablePassing(inlineSym).transform(
-          new MakeInlineableDirect(inlineSym).transform(tree))
+        // Make sure the old accessors are generated for binary compatibility
+        new MakeInlineablePassing(inlineSym, useBinaryAPI = false).transform(
+          new MakeInlineableDirect(inlineSym, useBinaryAPI = false).transform(tree))
+
+        // TODO: warn if MakeInlineablePassing or MakeInlineableDirect generate accessors when useBinaryAPI in enabled
+        new MakeInlineablePassing(inlineSym, useBinaryAPI = true).transform(
+          new MakeInlineableDirect(inlineSym, useBinaryAPI = true).transform(tree))
     }
   }
 

compiler/src/dotty/tools/dotc/transform/BinaryAPIAnnotations.scala

@@ -0,0 +1,36 @@
+package dotty.tools.dotc
+package transform
+
+import core._
+import ast.tpd._
+import Contexts._
+import MegaPhase._
+import Annotations._
+import Symbols.defn
+import Constants._
+import Types._
+import Symbols.*
+import Decorators._
+import Flags._
+import DenotTransformers.SymTransformer
+import SymDenotations.SymDenotation
+
+/** Makes @binaryAPI definitions public */
+class BinaryAPIAnnotations extends MiniPhase with SymTransformer:
+
+  override def phaseName: String = BinaryAPIAnnotations.name
+
+  override def description: String = BinaryAPIAnnotations.description
+
+  def transformSym(d: SymDenotation)(using Context): SymDenotation =
+    if d.isBinaryAPI then
+      d.resetFlag(Protected)
+      d.setPrivateWithin(NoSymbol)
+    d
+
+
+end BinaryAPIAnnotations
+
+object BinaryAPIAnnotations:
+  val name: String = "binaryAPIAnnotations"
+  val description: String = "makes @binaryAPI definitions public"

compiler/src/dotty/tools/dotc/transform/Memoize.scala

@@ -167,7 +167,10 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
             if isErasableBottomField(field, rhsClass) then erasedBottomTree(rhsClass)
             else transformFollowingDeep(ref(field))(using ctx.withOwner(sym))
           val getterDef = cpy.DefDef(tree)(rhs = getterRhs)
-          sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot))
+          val binaryAPIAnnotOpt = sym.getAnnotation(defn.BinaryAPIAnnot)
+          // FIXME: copyAndKeepAnnotationsCarrying is dropping defn.BinaryAPIAnnot
+          sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.BinaryAPIAnnot))
+          for binaryAPIAnnot <- binaryAPIAnnotOpt do sym.addAnnotation(binaryAPIAnnot)
           Thicket(fieldDef, getterDef)
       else if sym.isSetter then
         if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs: @unchecked } // This is intended as an assertion
@@ -193,7 +196,10 @@ class Memoize extends MiniPhase with IdentityDenotTransformer { thisPhase =>
             then Literal(Constant(()))
             else Assign(ref(field), adaptToField(field, ref(tree.termParamss.head.head.symbol)))
           val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(using ctx.withOwner(sym)))
-          sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.SetterMetaAnnot))
+          val binaryAPIAnnotOpt = sym.getAnnotation(defn.BinaryAPIAnnot)
+          // FIXME: copyAndKeepAnnotationsCarrying is dropping defn.BinaryAPIAnnot
+          sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.SetterMetaAnnot, defn.BinaryAPIAnnot))
+          for binaryAPIAnnot <- binaryAPIAnnotOpt do sym.addAnnotation(binaryAPIAnnot)
           setterDef
       else
         // Curiously, some accessors from Scala2 have ' ' suffixes.

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

@@ -159,10 +159,13 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
             if sym.isSetter then
               sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.SetterMetaAnnot))
           else
+            val binaryAPIAnnotOpt = sym.getAnnotation(defn.BinaryAPIAnnot)
             if sym.is(Param) then
               sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
             else if sym.is(ParamAccessor) then
-              sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot))
+              // FIXME: copyAndKeepAnnotationsCarrying is dropping defn.BinaryAPIAnnot
+              sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot, defn.BinaryAPIAnnot))
+              for binaryAPIAnnot <- binaryAPIAnnotOpt do sym.addAnnotation(binaryAPIAnnot)
             else
               sym.copyAndKeepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
           if sym.isScala2Macro && !ctx.settings.XignoreScala2Macros.value then

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

@@ -526,6 +526,11 @@ object Checking {
         fail(em"Inline methods cannot be @tailrec")
     if sym.hasAnnotation(defn.TargetNameAnnot) && sym.isClass && sym.isTopLevelClass then
       fail(TargetNameOnTopLevelClass(sym))
+    if sym.hasAnnotation(defn.BinaryAPIAnnot) then
+      if sym.is(Enum) then fail(em"@binaryAPI cannot be used on enum definitions.")
+      else if sym.isType && !(sym.is(Given) || sym.companionModule.is(Given)) then fail(em"@binaryAPI cannot be used on ${sym.showKind} definitions")
+      else if sym.isLocal && !sym.is(Param) then fail(em"@binaryAPI cannot be used on local definitions.")
+      else if sym.is(Private) then fail(em"@binaryAPI cannot be used on private definitions.\n\nCould use private[${sym.owner.name}] or protected instead.")
     if (sym.hasAnnotation(defn.NativeAnnot)) {
       if (!sym.is(Deferred))
         fail(NativeMembersMayNotHaveImplementation(sym))

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

@@ -99,7 +99,9 @@ trait TypeAssigner {
     val tpe1 = accessibleType(tpe, superAccess)
     if tpe1.exists then tpe1
     else tpe match
-      case tpe: NamedType => inaccessibleErrorType(tpe, superAccess, pos)
+      case tpe: NamedType =>
+        if tpe.termSymbol.isBinaryAPI then tpe
+        else inaccessibleErrorType(tpe, superAccess, pos)
       case NoType => tpe
 
   /** Return a potentially skolemized version of `qualTpe` to be used

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

@@ -1682,6 +1682,62 @@ class DottyBytecodeTests extends DottyBytecodeTest {
       assertSameCode(instructions, expected)
     }
   }
+
+  @Test
+  def i15413(): Unit = {
+    val code =
+      """import scala.quoted.*
+        |import scala.annotation.binaryAPI
+        |class Macro:
+        |  inline def foo = Macro.fooImpl
+        |  def test = foo
+        |object Macro:
+        |  @binaryAPI private[Macro] def fooImpl = {}
+      """.stripMargin
+    checkBCode(code) { dir =>
+      val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
+
+      // For 3.0-3.3 compat
+      val macroClass = loadClassNode(dir.lookupName("Macro.class", directory = false).input, skipDebugInfo = false)
+      val oldAccessor = getMethod(macroClass, "Macro$$inline$fooImpl")
+      assert(oldAccessor.signature == "()V")
+      assert((oldAccessor.access & privateAccessors) == 0)
+
+      // Check that the @binaryAPI annotated method is called
+      val testMethod = getMethod(macroClass, "test")
+      val testInstructions = instructionsFromMethod(testMethod).filter(_.isInstanceOf[Invoke])
+      assertSameCode(testInstructions, List(
+        Invoke(INVOKEVIRTUAL, "Macro$", "fooImpl", "()V", false)))
+    }
+  }
+
+  @Test
+  def i15413b(): Unit = {
+    val code =
+      """package foo
+        |import scala.annotation.binaryAPI
+        |class C:
+        |  inline def baz = D.bazImpl
+        |  def test = baz
+        |object D:
+        |  @binaryAPI private[foo] def bazImpl = {}
+      """.stripMargin
+    checkBCode(code) { dir =>
+      val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
+
+      // For 3.0-3.3 compat
+      val barClass = loadClassNode(dir.subdirectoryNamed("foo").lookupName("C.class", directory = false).input, skipDebugInfo = false)
+      val oldAccessor  = getMethod(barClass, "inline$bazImpl$i1")
+      assert(oldAccessor.desc == "(Lfoo/D$;)V", oldAccessor.desc)
+      assert((oldAccessor.access & privateAccessors) == 0)
+
+      // Check that the @binaryAPI annotated method is called
+      val testMethod = getMethod(barClass, "test")
+      val testInstructions = instructionsFromMethod(testMethod).filter(_.isInstanceOf[Invoke])
+      assertSameCode(testInstructions, List(
+        Invoke(INVOKEVIRTUAL, "foo/D$", "bazImpl", "()V", false)))
+    }
+  }
 }
 
 object invocationReceiversTestCode {

library/src/scala/annotation/binaryAPI.scala

@@ -0,0 +1,5 @@
+package scala.annotation
+
+/** TODO */
+@experimental
+class binaryAPI extends scala.annotation.StaticAnnotation

tests/neg/binaryAPI.check

@@ -0,0 +1,44 @@
+-- Error: tests/neg/binaryAPI.scala:6:18 -------------------------------------------------------------------------------
+6 |@binaryAPI object B // error // TODO support?
+  |                  ^
+  |                  @binaryAPI cannot be used on object definitions
+-- Error: tests/neg/binaryAPI.scala:7:17 -------------------------------------------------------------------------------
+7 |@binaryAPI class C: // error
+  |                 ^
+  |                 @binaryAPI cannot be used on class definitions
+-- Error: tests/neg/binaryAPI.scala:8:25 -------------------------------------------------------------------------------
+8 |  @binaryAPI private def f: Unit = // error
+  |                         ^
+  |                         @binaryAPI cannot be used on private definitions.
+  |
+  |                         Could use private[C] or protected instead.
+-- Error: tests/neg/binaryAPI.scala:9:19 -------------------------------------------------------------------------------
+9 |    @binaryAPI def g = () // error
+  |                   ^
+  |                   @binaryAPI cannot be used on local definitions.
+-- Error: tests/neg/binaryAPI.scala:11:19 ------------------------------------------------------------------------------
+11 |class D(@binaryAPI x: Int) // error
+   |                   ^
+   |                   @binaryAPI cannot be used on private definitions.
+   |
+   |                   Could use private[D] or protected instead.
+-- Error: tests/neg/binaryAPI.scala:12:19 ------------------------------------------------------------------------------
+12 |class E[@binaryAPI T] // error
+   |                   ^
+   |                   @binaryAPI cannot be used on type definitions
+-- Error: tests/neg/binaryAPI.scala:14:16 ------------------------------------------------------------------------------
+14 |@binaryAPI enum Enum1: // error
+   |                ^
+   |                @binaryAPI cannot be used on enum definitions.
+-- Error: tests/neg/binaryAPI.scala:18:18 ------------------------------------------------------------------------------
+18 |  @binaryAPI case A // error
+   |                  ^
+   |                  @binaryAPI cannot be used on enum definitions.
+-- Error: tests/neg/binaryAPI.scala:19:18 ------------------------------------------------------------------------------
+19 |  @binaryAPI case B(a: Int) // error
+   |                  ^
+   |                  @binaryAPI cannot be used on enum definitions.
+-- Error: tests/neg/binaryAPI.scala:5:16 -------------------------------------------------------------------------------
+5 |@binaryAPI type A // error
+  |                ^
+  |                @binaryAPI cannot be used on type definitions

tests/neg/binaryAPI.scala

@@ -0,0 +1,19 @@
+package foo
+
+import scala.annotation.binaryAPI
+
+@binaryAPI type A // error
+@binaryAPI object B // error // TODO support?
+@binaryAPI class C: // error
+  @binaryAPI private def f: Unit = // error
+    @binaryAPI def g = () // error
+    ()
+class D(@binaryAPI x: Int) // error
+class E[@binaryAPI T] // error
+
+@binaryAPI enum Enum1: // error
+  case A
+
+enum Enum2:
+  @binaryAPI case A // error
+  @binaryAPI case B(a: Int) // error

tests/pos-macros/i15413/Macro_1.scala

@@ -0,0 +1,8 @@
+import scala.quoted.*
+import scala.annotation.binaryAPI
+
+class Macro:
+  inline def foo = ${ Macro.fooImpl }
+
+object Macro:
+  @binaryAPI private[Macro] def fooImpl(using Quotes) = '{}

tests/pos-macros/i15413/Test_2.scala

@@ -0,0 +1,2 @@
+def test =
+  new Macro().foo

tests/pos-macros/i15413b/Macro_1.scala

@@ -0,0 +1,8 @@
+package bar
+
+import scala.quoted.*
+import scala.annotation.binaryAPI
+
+inline def foo = ${ fooImpl }
+
+@binaryAPI private[bar] def fooImpl(using Quotes) = '{}

tests/pos-macros/i15413b/Test_2.scala

@@ -0,0 +1 @@
+def test = bar.foo