Add @binaryapi

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

Author: nicolasstucki

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,25 @@
+package dotty.tools.dotc.transform
+
+import dotty.tools.dotc.core.Contexts.*
+import dotty.tools.dotc.core.DenotTransformers.SymTransformer
+import dotty.tools.dotc.core.Flags.*
+import dotty.tools.dotc.core.Symbols.NoSymbol
+import dotty.tools.dotc.core.SymDenotations.SymDenotation
+import dotty.tools.dotc.transform.MegaPhase.MiniPhase
+
+/** 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
+  }
+
+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