Warn if inline accessors are generated

Author: nicolasstucki

community-build/community-projects/scodec

@@ -9,6 +9,8 @@ import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.report
 import dotty.tools.dotc.core.Phases
 
+import scala.annotation.binaryAPI
+
 /**
  * Functionality needed in the post-processor whose implementation depends on the compiler
  * frontend. All methods are synchronized.
@@ -20,6 +22,7 @@ sealed abstract class PostProcessorFrontendAccess {
   def backendReporting: BackendReporting
   def getEntryPoints: List[String]
 
+  @binaryAPI
   private val frontendLock: AnyRef = new Object()
   inline final def frontendSynch[T](inline x: => T): T = frontendLock.synchronized(x)
 }

compiler/src/dotty/tools/backend/jvm/PostProcessorFrontendAccess.scala

@@ -15,6 +15,8 @@ import NameKinds.AvoidNameKind
 import util.SimpleIdentitySet
 import NullOpsDecorator.stripNull
 
+import scala.annotation.binaryAPI
+
 /** Methods for adding constraints and solving them.
  *
  * What goes into a Constraint as opposed to a ConstrainHandler?
@@ -39,12 +41,12 @@ trait ConstraintHandling {
   private var addConstraintInvocations = 0
 
   /** If the constraint is frozen we cannot add new bounds to the constraint. */
-  protected var frozenConstraint: Boolean = false
+  @binaryAPI protected var frozenConstraint: Boolean = false
 
   /** Potentially a type lambda that is still instantiatable, even though the constraint
    *  is generally frozen.
    */
-  protected var caseLambda: Type = NoType
+  @binaryAPI protected var caseLambda: Type = NoType
 
   /** If set, align arguments `S1`, `S2`when taking the glb
    *  `T1 { X = S1 } & T2 { X = S2 }` of a constraint upper bound for some type parameter.
@@ -56,7 +58,7 @@ trait ConstraintHandling {
   /** We are currently comparing type lambdas. Used as a flag for
    *  optimization: when `false`, no need to do an expensive `pruneLambdaParams`
    */
-  protected var comparedTypeLambdas: Set[TypeLambda] = Set.empty
+  @binaryAPI protected var comparedTypeLambdas: Set[TypeLambda] = Set.empty
 
   /** Used for match type reduction: If false, we don't recognize an abstract type
    *  to be a subtype type of any of its base classes. This is in place only at the
@@ -110,7 +112,7 @@ trait ConstraintHandling {
    *  of `1`. So the lower bound is `1 | x.M` and when we level-avoid that we
    *  get `1 | Int & String`, which simplifies to `Int`.
    */
-  private var myTrustBounds = true
+  @binaryAPI private var myTrustBounds = true
 
   inline def withUntrustedBounds(op: => Type): Type =
     val saved = myTrustBounds

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

@@ -30,6 +30,7 @@ import classfile.ReusableDataReader
 import StdNames.nme
 import compiletime.uninitialized
 
+import scala.annotation.binaryAPI
 import scala.annotation.internal.sharable
 
 import DenotTransformers.DenotTransformer
@@ -805,6 +806,7 @@ object Contexts {
    *  Note: plain TypeComparers always take on the kind of the outer comparer if they are in the same context.
    *  In other words: tracking or explaining is a sticky property in the same context.
    */
+  @binaryAPI
   private def comparer(using Context): TypeComparer =
     util.Stats.record("comparing")
     val base = ctx.base
@@ -980,7 +982,7 @@ object Contexts {
     private[core] var phasesPlan: List[List[Phase]] = uninitialized
 
     /** Phases by id */
-    private[dotc] var phases: Array[Phase] = uninitialized
+    @binaryAPI private[dotc] var phases: Array[Phase] = uninitialized
 
     /** Phases with consecutive Transforms grouped into a single phase, Empty array if fusion is disabled */
     private[core] var fusedPhases: Array[Phase] = Array.empty[Phase]
@@ -1019,7 +1021,7 @@ object Contexts {
     val generalContextPool = ContextPool()
 
     private[Contexts] val comparers = new mutable.ArrayBuffer[TypeComparer]
-    private[Contexts] var comparersInUse: Int = 0
+    @binaryAPI private[Contexts] var comparersInUse: Int = 0
 
     private var charArray = new Array[Char](256)
 

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

@@ -25,6 +25,8 @@ import reporting.trace
 import annotation.constructorOnly
 import cc.{CapturingType, derivedCapturingType, CaptureSet, stripCapturing, isBoxedCapturing, boxed, boxedUnlessFun, boxedIfTypeParam, isAlwaysPure}
 
+import scala.annotation.binaryAPI
+
 /** Provides methods to compare types.
  */
 class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling, PatternTypeConstrainer {
@@ -34,7 +36,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
   private var myContext: Context = initctx
   def comparerContext: Context = myContext
 
-  protected given [DummySoItsADef]: Context = myContext
+  @binaryAPI protected given [DummySoItsADef]: Context = myContext
 
   protected var state: TyperState = compiletime.uninitialized
   def constraint: Constraint = state.constraint
@@ -115,7 +117,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
 
   private def isBottom(tp: Type) = tp.widen.isRef(NothingClass)
 
-  protected def gadtBounds(sym: Symbol)(using Context) = ctx.gadt.bounds(sym)
+  @binaryAPI protected def gadtBounds(sym: Symbol)(using Context) = ctx.gadt.bounds(sym)
   protected def gadtAddBound(sym: Symbol, b: Type, isUpper: Boolean): Boolean = ctx.gadtState.addBound(sym, b, isUpper)
 
   protected def typeVarInstance(tvar: TypeVar)(using Context): Type = tvar.underlying
@@ -156,7 +158,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
   private [this] var leftRoot: Type | Null = null
 
   /** Are we forbidden from recording GADT constraints? */
-  private var frozenGadt = false
+  @binaryAPI private var frozenGadt = false
   private inline def inFrozenGadt[T](inline op: T): T =
     inFrozenGadtIf(true)(op)
 
@@ -187,7 +189,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
     inFrozenGadtIf(true)(inFrozenConstraint(op))
 
   extension (sym: Symbol)
-    private inline def onGadtBounds(inline op: TypeBounds => Boolean): Boolean =
+    @binaryAPI private inline def onGadtBounds(inline op: TypeBounds => Boolean): Boolean =
       val bounds = gadtBounds(sym)
       bounds != null && op(bounds)
 

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

@@ -39,6 +39,7 @@ import compiletime.uninitialized
 import cc.{CapturingType, CaptureSet, derivedCapturingType, isBoxedCapturing, EventuallyCapturingType, boxedUnlessFun}
 import CaptureSet.{CompareResult, IdempotentCaptRefMap, IdentityCaptRefMap}
 
+import scala.annotation.binaryAPI
 import scala.annotation.internal.sharable
 import scala.annotation.threadUnsafe
 
@@ -5542,7 +5543,7 @@ object Types {
 
   /** Common base class of TypeMap and TypeAccumulator */
   abstract class VariantTraversal:
-    protected[dotc] var variance: Int = 1
+    @binaryAPI protected[dotc] var variance: Int = 1
 
     inline protected def atVariance[T](v: Int)(op: => T): T = {
       val saved = variance

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

@@ -21,6 +21,7 @@ import transform.{AccessProxies, Splicer}
 import staging.CrossStageSafety
 import transform.SymUtils.*
 import config.Printers.inlining
+import util.SrcPos
 import util.Property
 import staging.StagingLevel
 
@@ -74,7 +75,7 @@ object PrepareInlineable {
       def needsAccessor(sym: Symbol)(using Context): Boolean =
         sym.isTerm &&
         (sym.isOneOf(AccessFlags) || sym.privateWithin.exists) &&
-        (!sym.isBinaryAPI || (sym.is(Private) && !sym.owner.is(Trait))) &&
+        (!sym.isBinaryAPI || sym.is(Private)) &&
         !(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
         !sym.isInlineMethod &&
         (Inlines.inInlineMethod || StagingLevel.level > 0)
@@ -100,6 +101,22 @@ object PrepareInlineable {
 
       override def transform(tree: Tree)(using Context): Tree =
         postTransform(super.transform(preTransform(tree)))
+
+      protected def unstableAccessorWarning(accessor: Symbol, accessed: Symbol, srcPos: SrcPos)(using Context): Unit =
+        val accessorDefTree = accessorDef(accessor, accessed)
+        val solution =
+          if accessed.is(Private) then
+            s"Annotate ${accessed.name} with `@binaryAPI` to generate a stable accessor."
+          else
+            s"Annotate ${accessed.name} with `@binaryAPI` to make it accessible."
+        val binaryCompat =
+          s"""Adding @binaryAPI may break binary compatibility if a previous version of this
+              |library was compiled with Scala 3.0-3.3, Binary compatibility should be checked
+              |using MiMa. To keep binary you can add the following accessor to ${accessor.owner.showKind} ${accessor.owner.name.stripModuleClassSuffix}:
+              |  @binaryAPI private[${accessor.owner.name.stripModuleClassSuffix}] ${accessorDefTree.show}
+              |
+              |""".stripMargin
+        report.warning(em"Generated unstable inline accessor for $accessed defined in ${accessed.owner}.\n\n$solution\n\n$binaryCompat", srcPos)
     }
 
     /** Direct approach: place the accessor with the accessed symbol. This has the
@@ -114,7 +131,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 accessorTree = useAccessor(tree)
+            if !tree.symbol.isBinaryAPI && tree.symbol != accessorTree.symbol then
+              unstableAccessorWarning(accessorTree.symbol, tree.symbol, tree.srcPos)
+            accessorTree
         case _ =>
           tree
       }
@@ -199,7 +220,9 @@ object PrepareInlineable {
             localRefs.map(TypeTree(_)) ++ leadingTypeArgs, // TODO: pass type parameters in two sections?
             (qual :: Nil) :: otherArgss
           )
-          ref(accessor).appliedToArgss(argss1).withSpan(tree.span)
+          val accessorTree = ref(accessor).appliedToArgss(argss1).withSpan(tree.span)
+
+          unstableAccessorWarning(accessorTree.symbol, tree.symbol, tree.srcPos)
 
             // TODO: Handle references to non-public types.
             // This is quite tricky, as such types can appear anywhere, including as parts
@@ -211,6 +234,7 @@ object PrepareInlineable {
             //  myAccessors += TypeDef(accessor).withPos(tree.pos.focus)
             //  ref(accessor).withSpan(tree.span)
             //
+          accessorTree
         case _: TypeDef if tree.symbol.is(Case) =>
           report.error(reporting.CaseClassInInlinedCode(tree), tree)
           tree
@@ -222,7 +246,7 @@ object PrepareInlineable {
     /** Create an inline accessor for this definition. */
     def makePrivateBinaryAPIAccessor(sym: Symbol)(using Context): Unit =
       assert(sym.is(Private))
-      if !sym.owner.is(Trait) then
+      if !sym.is(Accessor) then
         val ref = tpd.ref(sym).asInstanceOf[RefTree]
         val accessor = InsertPrivateBinaryAPIAccessors.useAccessor(ref)
         if sym.is(Mutable) then
@@ -243,7 +267,6 @@ object PrepareInlineable {
         // so no accessors are needed for them.
         tree
       else
-        // TODO: warn if MakeInlineablePassing or MakeInlineableDirect generate accessors
         new MakeInlineablePassing(inlineSym).transform(
           new MakeInlineableDirect(inlineSym).transform(tree))
     }

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

@@ -21,6 +21,8 @@ import config.Feature.{migrateTo3, fewerBracesEnabled}
 import config.SourceVersion.`3.0`
 import reporting.{NoProfile, Profile, Message}
 
+import scala.annotation.binaryAPI
+
 import java.util.Objects
 
 object Scanners {
@@ -1597,7 +1599,7 @@ object Scanners {
     protected def coversIndent(w: IndentWidth): Boolean =
       knownWidth != null && w == indentWidth
 
-    private var myCommasExpected: Boolean = false
+    @binaryAPI private var myCommasExpected: Boolean = false
 
     inline def withCommasExpected[T](inline op: => T): T =
       val saved = myCommasExpected

compiler/src/dotty/tools/dotc/parsing/Scanners.scala

@@ -19,6 +19,7 @@ import Decorators.{em, toMessage}
 import util.SourceFile
 import Utility._
 
+import scala.annotation.binaryAPI
 
 // XXX/Note: many/most of the functions in here are almost direct cut and pastes
 // from another file - scala.xml.parsing.MarkupParser, it looks like.
@@ -52,7 +53,7 @@ object MarkupParsers {
     override def getMessage: String = "input ended while parsing XML"
   }
 
-  class MarkupParser(parser: Parser, final val preserveWS: Boolean)(using Context) extends MarkupParserCommon {
+  class MarkupParser(@binaryAPI parser: Parser, final val preserveWS: Boolean)(using @binaryAPI c: Context) extends MarkupParserCommon {
 
     import Tokens.{ LBRACE, RBRACE }
 
@@ -93,8 +94,8 @@ object MarkupParsers {
     var xEmbeddedBlock: Boolean = false
 
     private var debugLastStartElement = List.empty[(Int, String)]
-    private def debugLastPos = debugLastStartElement.head._1
-    private def debugLastElem = debugLastStartElement.head._2
+    @binaryAPI private def debugLastPos = debugLastStartElement.head._1
+    @binaryAPI private def debugLastElem = debugLastStartElement.head._2
 
     private def errorBraces() = {
       reportSyntaxError("in XML content, please use '}}' to express '}'")

compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala

@@ -31,11 +31,13 @@ import dotty.tools.dotc.util.SourcePosition
 import dotty.tools.dotc.ast.untpd.{MemberDef, Modifiers, PackageDef, RefTree, Template, TypeDef, ValOrDefDef}
 import cc.{CaptureSet, toCaptureSet, IllegalCaptureRef}
 
+import scala.annotation.binaryAPI
+
 class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
 
   /** A stack of enclosing DefDef, TypeDef, or ClassDef, or ModuleDefs nodes */
   private var enclosingDef: untpd.Tree = untpd.EmptyTree
-  private var myCtx: Context = super.printerContext
+  @binaryAPI private var myCtx: Context = super.printerContext
   private var printPos = ctx.settings.YprintPos.value
   private val printLines = ctx.settings.printLines.value
 

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala

@@ -8,6 +8,7 @@ import core.*, Contexts.*, Decorators.*
 import config.*
 import printing.Formatting.*
 
+import scala.annotation.binaryAPI
 import scala.compiletime.*
 
 /** Exposes the {{{ trace("question") { op } }}} syntax.
@@ -76,9 +77,9 @@ trait TraceSyntax:
   inline def apply[T](inline question: String)(inline op: T)(using Context): T =
     apply[T](question, false)(op)
 
-  private val alwaysToString = (x: Any) => String.valueOf(x)
+  @binaryAPI private val alwaysToString = (x: Any) => String.valueOf(x)
 
-  private def doTrace[T](question: => String,
+  @binaryAPI private def doTrace[T](question: => String,
                          printer: Printers.Printer = Printers.default,
                          showOp: T => String)
                         (op: => T)(using Context): T =

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

@@ -32,29 +32,33 @@ 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 => {
-        def numTypeParams = accessed.info match {
-          case info: PolyType => info.paramNames.length
-          case _ => 0
-        }
-        val (targs, argss) = splitArgs(prefss)
-        val (accessRef, forwardedTpts, forwardedArgss) =
-          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)),
-             targs, argss)
-        val rhs =
-          if (accessor.name.isSetterName &&
-              forwardedArgss.nonEmpty && forwardedArgss.head.nonEmpty) // defensive conditions
-            accessRef.becomes(forwardedArgss.head.head)
-          else
-            accessRef
-              .appliedToTypeTrees(forwardedTpts)
-              .appliedToArgss(forwardedArgss)
-              .etaExpandCFT(using ctx.withOwner(accessor))
-        rhs.withSpan(accessed.span)
-      })
+      accessorDef(accessor, accessed)
+
+  /** The accessor definitions that need to be added to class `cls` */
+  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
+      }
+      val (targs, argss) = splitArgs(prefss)
+      val (accessRef, forwardedTpts, forwardedArgss) =
+        if (passReceiverAsArg(accessor.name))
+          (argss.head.head.select(accessed), targs.takeRight(numTypeParams), argss.tail)
+        else
+          (if (accessed.isStatic) ref(accessed) else ref(TermRef(accessor.owner.thisType, accessed)),
+            targs, argss)
+      val rhs =
+        if (accessor.name.isSetterName &&
+            forwardedArgss.nonEmpty && forwardedArgss.head.nonEmpty) // defensive conditions
+          accessRef.becomes(forwardedArgss.head.head)
+        else
+          accessRef
+            .appliedToTypeTrees(forwardedTpts)
+            .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] = {