Add @publicInBinary annotation and -WunstableInlineAccessors

Introduces [SIP-52](scala/improvement-proposals#58) as experimental feature.

A binary API is a definition that is annotated with `@publicInBinary` or overrides a definition annotated with `@publicInBinary`. 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 `@publicInBinary` 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 `-WunstableInlineAccessors` is enabled, then a warning will be emitted if an inline accessor is generated. The warning will guide the user to the use of `@publicInBinary`.

Author: nicolasstucki

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

@@ -166,6 +166,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 WunstableInlineAccessors = BooleanSetting("-WunstableInlineAccessors", "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

@@ -1060,6 +1060,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 PublicInBinaryAnnot: ClassSymbol = requiredClass("scala.annotation.publicInBinary")
 
   @tu lazy val JavaRepeatableAnnot: ClassSymbol = requiredClass("java.lang.annotation.Repeatable")
 
@@ -1071,6 +1072,8 @@ class Definitions {
   // A list of meta-annotations that are relevant for fields and accessors
   @tu lazy val NonBeanMetaAnnots: Set[Symbol] =
     Set(FieldMetaAnnot, GetterMetaAnnot, ParamMetaAnnot, SetterMetaAnnot, CompanionClassMetaAnnot, CompanionMethodMetaAnnot)
+  @tu lazy val NonBeanParamAccessorAnnots: Set[Symbol] =
+    Set(PublicInBinaryAnnot)
   @tu lazy val MetaAnnots: Set[Symbol] =
     NonBeanMetaAnnots + BeanGetterMetaAnnot + BeanSetterMetaAnnot
 

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

@@ -1031,6 +1031,13 @@ object SymDenotations {
       isOneOf(EffectivelyErased)
       || is(Inline) && !isRetainedInline && !hasAnnotation(defn.ScalaStaticAnnot)
 
+    /** 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))
+      )
+
     /** ()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

@@ -22,6 +22,8 @@ import staging.CrossStageSafety
 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.*
@@ -71,6 +73,7 @@ object PrepareInlineable {
         sym.isTerm &&
         (sym.isOneOf(AccessFlags) || sym.privateWithin.exists) &&
         !sym.isContainedIn(inlineSym) &&
+        !sym.hasPublicInBinary &&
         !(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
         !sym.isInlineMethod &&
         (Inlines.inInlineMethod || StagingLevel.level > 0)
@@ -86,6 +89,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.WunstableInlineAccessors.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
@@ -100,7 +108,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
       }
@@ -179,6 +191,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/printing/RefinedPrinter.scala

@@ -121,14 +121,19 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
       else if homogenizedView then isEmptyPrefix(sym) // drop <root> and anonymous classes, but not scala, Predef.
       else if sym.isPackageObject then isOmittablePrefix(sym.owner)
       else isOmittablePrefix(sym)
+    def isSkippedPackageObject(sym: Symbol) =
+      sym.isPackageObject && !homogenizedView && !printDebug
 
     tp.prefix match {
-      case thisType: ThisType if isOmittable(thisType.cls) =>
-        ""
-      case termRef @ TermRef(pre, _) =>
+      case thisType: ThisType =>
+        val sym = thisType.cls
+        if isSkippedPackageObject(sym) then toTextPrefixOf(sym.typeRef)
+        else if isOmittable(sym) then ""
+        else super.toTextPrefixOf(tp)
+      case termRef: TermRef =>
         val sym = termRef.symbol
-        if sym.isPackageObject && !homogenizedView && !printDebug then toTextPrefixOf(termRef)
-        else if (isOmittable(sym)) ""
+        if isSkippedPackageObject(sym) then toTextPrefixOf(termRef)
+        else if isOmittable(sym) then ""
         else super.toTextPrefixOf(tp)
       case _ => super.toTextPrefixOf(tp)
     }

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

@@ -204,6 +204,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case VarArgsParamCannotBeGivenID // errorNumber: 188
   case ExtractorNotFoundID // errorNumber: 189
   case PureUnitExpressionID // errorNumber: 190
+  case UnstableInlineAccessorID // errorNumber: 191
 
   def errorNumber = ordinal - 1
 

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

@@ -3087,3 +3087,40 @@ 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 @publicInBinary
+       | * 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:
+       |  @publicInBinary private[$within] ${accessorTree.show}
+       |"""
+
+  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

@@ -31,7 +31,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
@@ -41,7 +45,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 &&
@@ -53,7 +57,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] = {
@@ -148,7 +153,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 @publicInBinary 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/PostTyper.scala

@@ -172,7 +172,10 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
             if sym.is(Param) then
               sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
             else if sym.is(ParamAccessor) then
+              // @publicInBinary is not a meta-annotation and therefore not kept by `keepAnnotationsCarrying`
+              val publicInBinaryAnnotOpt = sym.getAnnotation(defn.PublicInBinaryAnnot)
               sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot))
+              for publicInBinaryAnnot <- publicInBinaryAnnotOpt do sym.addAnnotation(publicInBinaryAnnot)
             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/transform/ProtectedAccessors.scala

@@ -37,7 +37,7 @@ object ProtectedAccessors {
    *  is not in a subclass or subtrait of `sym`?
    */
   def needsAccessorIfNotInSubclass(sym: Symbol)(using Context): Boolean =
-    sym.isTerm && sym.is(Protected) &&
+    sym.isTerm && sym.is(Protected) && !sym.hasPublicInBinary &&
     !sym.owner.is(Trait) && // trait methods need to be handled specially, are currently always public
     !insideBoundaryOf(sym)
 

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

@@ -538,6 +538,12 @@ 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.PublicInBinaryAnnot) then
+      if sym.is(Enum) then fail(em"@publicInBinary cannot be used on enum definitions")
+      else if sym.isType && !sym.is(Module) && !(sym.is(Given) || sym.companionModule.is(Given)) then fail(em"@publicInBinary cannot be used on ${sym.showKind} definitions")
+      else if !sym.owner.isClass && !(sym.is(Param) && sym.owner.isConstructor) then fail(em"@publicInBinary cannot be used on local definitions")
+      else if sym.is(ParamAccessor) && sym.is(Private) then fail(em"@publicInBinary cannot be non `val` constructor parameters")
+      else if sym.is(Private) && !sym.privateWithin.exists && !sym.isConstructor then fail(em"@publicInBinary cannot be used on private definitions\n\nConsider using `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,10 @@ 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.hasPublicInBinary && tpd.enclosingInlineds.nonEmpty then tpe
+        else inaccessibleErrorType(tpe, superAccess, pos)
+      case _ => tpe
 
   /** Return a potentially skolemized version of `qualTpe` to be used
    *  as a prefix when selecting `name`.

compiler/test-resources/repl/i15493

@@ -146,4 +146,4 @@ scala> Vector.unapplySeq(Vector(2))
 val res35: scala.collection.SeqFactory.UnapplySeqWrapper[Int] = scala.collection.SeqFactory$UnapplySeqWrapper@df507bfd
 
 scala> new scala.concurrent.duration.DurationInt(5)
-val res36: scala.concurrent.duration.package.DurationInt = scala.concurrent.duration.package$DurationInt@5
+val res36: scala.concurrent.duration.DurationInt = scala.concurrent.duration.package$DurationInt@5