Add @binaryAPI annotation

A binary API is a definition that is annotated with `@binaryAPI` or overrides a definition annotated with `@binaryAPI`. This annotation can be placed on `def`, `val`, `lazy val`, `var`, `object`, and `given` definitions. A binary API will be publicly available in the bytecode. It cannot be used on `private`/`private[this]` definitions.

This is useful in combination with inline definitions. If an inline definition refers to a private/protected definition marked as `@binaryAPI` it does not need to use an accessor. We still generate the accessors for binary compatibility but do not use them.

If the linting option `-WunstableInlineAPI` is enabled, then a warning will be emitted if an inline accessor is generated. The warning will guide the user to the use of `@binaryAPI`.

For SIP: scala/improvement-proposals#58

Author: nicolasstucki

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

@@ -72,7 +72,8 @@ class Compiler {
          new ElimRepeated,           // Rewrite vararg parameters and arguments
          new RefChecks) ::           // Various checks mostly related to abstract members and overriding
     List(new init.Checker) ::        // Check initialization of objects
-    List(new ProtectedAccessors,     // Add accessors for protected members
+    List(new BinaryAPIAnnotations,   // Makes @binaryAPI definitions public
+         new ProtectedAccessors,     // Add accessors for protected members
          new ExtensionMethods,       // Expand methods of value classes with extension methods
          new UncacheGivenAliases,    // Avoid caching RHS of simple parameterless given aliases
          new ElimByName,             // Map by-name parameters to functions

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -163,6 +163,7 @@ private sealed trait WarningSettings:
   val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
   val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
   val WimplausiblePatterns = BooleanSetting("-Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
+  val WunstableInlineAPI = BooleanSetting("-WunstableInlineAPI", "Warn an inline methods has references to non-stable binary APIs.")
   val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
     name = "-Wunused",
     helpArg = "warning",

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

@@ -1041,6 +1041,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(sym => sym.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

@@ -23,6 +23,8 @@ import transform.SymUtils.*
 import config.Printers.inlining
 import util.Property
 import staging.StagingLevel
+import dotty.tools.dotc.reporting.Message
+import dotty.tools.dotc.util.SrcPos
 
 object PrepareInlineable {
   import tpd._
@@ -72,6 +74,7 @@ object PrepareInlineable {
         sym.isTerm &&
         (sym.isOneOf(AccessFlags) || sym.privateWithin.exists) &&
         !sym.isContainedIn(inlineSym) &&
+        !sym.isBinaryAPI &&
         !(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
         !sym.isInlineMethod &&
         (Inlines.inInlineMethod || StagingLevel.level > 0)
@@ -87,6 +90,11 @@ object PrepareInlineable {
 
       override def transform(tree: Tree)(using Context): Tree =
         postTransform(super.transform(preTransform(tree)))
+
+      protected def checkUnstableAccessor(accessedTree: Tree, accessor: Symbol)(using Context): Unit =
+        if ctx.settings.WunstableInlineAPI.value then
+          val accessorTree = accessorDef(accessor, accessedTree.symbol)
+          report.warning(reporting.UnstableInlineAccessor(accessedTree.symbol, accessorTree), accessedTree)
     }
 
     /** Direct approach: place the accessor with the accessed symbol. This has the
@@ -101,7 +109,11 @@ object PrepareInlineable {
             report.error("Implementation restriction: cannot use private constructors in inline methods", tree.srcPos)
             tree // TODO: create a proper accessor for the private constructor
           }
-          else useAccessor(tree)
+          else
+            val accessor = useAccessor(tree)
+            if tree != accessor then
+              checkUnstableAccessor(tree, accessor.symbol)
+            accessor
         case _ =>
           tree
       }
@@ -180,6 +192,8 @@ object PrepareInlineable {
             accessorInfo = abstractQualType(addQualType(dealiasMap(accessedType))),
             accessed = accessed)
 
+          checkUnstableAccessor(tree, accessor)
+
           val (leadingTypeArgs, otherArgss) = splitArgs(argss)
           val argss1 = joinArgs(
             localRefs.map(TypeTree(_)) ++ leadingTypeArgs, // TODO: pass type parameters in two sections?

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -199,9 +199,10 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case ClosureCannotHaveInternalParameterDependenciesID // errorNumber: 183
   case MatchTypeNoCasesID // errorNumber: 184
   case UnimportedAndImportedID // errorNumber: 185
-  case ImplausiblePatternWarningID // erorNumber: 186
+  case ImplausiblePatternWarningID // errorNumber: 186
   case SynchronizedCallOnBoxedClassID // errorNumber: 187
-  case VarArgsParamCannotBeGivenID // erorNumber: 188
+  case VarArgsParamCannotBeGivenID // errorNumber: 188
+  case UnstableInlineAccessorID // errorNumber: 189
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -876,7 +876,7 @@ extends Message(PatternMatchExhaustivityID) {
 
     val pathes = List(
       ActionPatch(
-        srcPos = endPos, 
+        srcPos = endPos,
         replacement = uncoveredCases.map(c => indent(s"case $c => ???", startColumn))
           .mkString("\n", "\n", "")
       ),
@@ -3038,3 +3038,43 @@ class ImplausiblePatternWarning(pat: tpd.Tree, selType: Type)(using Context)
           |$pat  could match selector of type  $selType
           |only if there is an `equals` method identifying elements of the two types."""
     def explain(using Context) = ""
+
+class UnstableInlineAccessor(accessed: Symbol, accessorTree: tpd.Tree)(using Context)
+  extends Message(UnstableInlineAccessorID) {
+  def kind = MessageKind.Compatibility
+
+  def msg(using Context) =
+    i"""Unstable inline accessor ${accessor.name} was generated in $where."""
+
+  def explain(using Context) =
+    i"""Access to non-public $accessed causes the automatic generation of an accessor.
+       |This accessor is not stable, its name may change or it may disappear
+       |if not needed in a future version.
+       |
+       |To make sure that the inlined code is binary compatible you must make sure that
+       |$accessed is public in the binary API.
+       | * Option 1: Annotate $accessed with @binaryAPI
+       | * Option 2: Make $accessed public
+       |
+       |This change may break binary compatibility if a previous version of this
+       |library was compiled with generated accessors. Binary compatibility should
+       |be checked using MiMa. If binary compatibility is broken, you should add the
+       |old accessor explicitly in the source code. The following code should be
+       |added to $where:
+       |  @binaryAPI private[$within] ${accessorTree.showIndented(2)}
+       |"""
+       // FIXME accessorTree is shown with package classes, see tests/neg/inline-unstable-accessors.check:
+       //  - @binaryAPI private[G] def inline$valBinaryAPI1: Int = foo.G.package.valBinaryAPI1
+       //  - @binaryAPI private[I] def inline$valBinaryAPI1: Int = foo.I.inline-unstable-accessors$package.valBinaryAPI1
+
+  private def accessor = accessorTree.symbol
+
+  private def where =
+    if accessor.owner.name.isPackageObjectName then s"package ${within}"
+    else if accessor.owner.is(Module) then s"object $within"
+    else s"class $within"
+
+  private def within =
+    if accessor.owner.name.isPackageObjectName then accessor.owner.owner.name.stripModuleClassSuffix
+    else accessor.owner.name.stripModuleClassSuffix
+}

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

@@ -32,7 +32,11 @@ abstract class AccessProxies {
   /** The accessor definitions that need to be added to class `cls` */
   private def accessorDefs(cls: Symbol)(using Context): Iterator[DefDef] =
     for accessor <- cls.info.decls.iterator; accessed <- accessedBy.get(accessor) yield
-      DefDef(accessor.asTerm, prefss => {
+      accessorDef(accessor, accessed)
+
+  protected def accessorDef(accessor: Symbol, accessed: Symbol)(using Context): DefDef =
+    DefDef(accessor.asTerm,
+      prefss => {
         def numTypeParams = accessed.info match {
           case info: PolyType => info.paramNames.length
           case _ => 0
@@ -42,7 +46,7 @@ abstract class AccessProxies {
           if (passReceiverAsArg(accessor.name))
             (argss.head.head.select(accessed), targs.takeRight(numTypeParams), argss.tail)
           else
-            (if (accessed.isStatic) ref(accessed) else ref(TermRef(cls.thisType, accessed)),
+            (if (accessed.isStatic) ref(accessed) else ref(TermRef(accessor.owner.thisType, accessed)),
              targs, argss)
         val rhs =
           if (accessor.name.isSetterName &&
@@ -54,7 +58,8 @@ abstract class AccessProxies {
               .appliedToArgss(forwardedArgss)
               .etaExpandCFT(using ctx.withOwner(accessor))
         rhs.withSpan(accessed.span)
-      })
+      }
+    )
 
   /** Add all needed accessors to the `body` of class `cls` */
   def addAccessorDefs(cls: Symbol, body: List[Tree])(using Context): List[Tree] = {
@@ -149,7 +154,7 @@ abstract class AccessProxies {
     def accessorIfNeeded(tree: Tree)(using Context): Tree = tree match {
       case tree: RefTree if needsAccessor(tree.symbol) =>
         if (tree.symbol.isConstructor) {
-          report.error("Implementation restriction: cannot use private constructors in inlineable methods", tree.srcPos)
+          report.error("Cannot use private constructors in inline methods. You can use @binaryAPI to make constructor accessible in inline methods.", tree.srcPos)
           tree // TODO: create a proper accessor for the private constructor
         }
         else useAccessor(tree)

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

@@ -0,0 +1,35 @@
+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
+import dotty.tools.dotc.typer.RefChecks
+
+/** Makes @binaryAPI definitions public.
+ *
+ *  This makes it possible to elide the generations of some unnecessary accessors.
+ */
+class BinaryAPIAnnotations extends MiniPhase with SymTransformer:
+
+  override def runsAfterGroupsOf: Set[String] = Set(RefChecks.name)
+
+  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)
+      if d.is(Module) then
+        val moduleClass = d.moduleClass
+        moduleClass.resetFlag(Protected)
+        moduleClass.setPrivateWithin(NoSymbol)
+    d
+  }
+
+object BinaryAPIAnnotations:
+  val name: String = "binaryAPIAnnotations"
+  val description: String = "makes @binaryAPI definitions public"

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

@@ -169,10 +169,13 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
                   atPhase(thisPhase)(cls.annotationsCarrying(Set(defn.CompanionMethodMetaAnnot)))
                     ++ sym.annotations)
           else
+            val binaryAPIAnnotOpt = sym.getAnnotation(defn.BinaryAPIAnnot)
             if sym.is(Param) then
               sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
             else if sym.is(ParamAccessor) then
-              sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot))
+              // FIXME: copyAndKeepAnnotationsCarrying is dropping defn.BinaryAPIAnnot
+              sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot, defn.BinaryAPIAnnot, defn.BinaryAPIAnnot))
+              for binaryAPIAnnot <- binaryAPIAnnotOpt do sym.addAnnotation(binaryAPIAnnot)
             else
               sym.keepAnnotationsCarrying(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

@@ -542,6 +542,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(Module) && !(sym.is(Given) || sym.companionModule.is(Given)) then fail(em"@binaryAPI cannot be used on ${sym.showKind} definitions")
+      else if !sym.owner.isClass && !(sym.is(Param) && sym.owner.isConstructor) then fail(em"@binaryAPI cannot be used on local definitions")
+      else if sym.is(Private) && !sym.privateWithin.exists && !sym.isConstructor then fail(em"@binaryAPI cannot be used on private definitions\n\nCould the definition `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

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