Skip to content

Commit

Permalink
Split into @binaryAPI and @binaryAPIAccessor
Browse files Browse the repository at this point in the history
* `@binaryAPI` makes the API public. Cannot be used on `private[this]`.
* `@binaryAPIAccessor` generates the private accessor.
  • Loading branch information
nicolasstucki committed Mar 23, 2023
1 parent f3636b5 commit d4472ae
Show file tree
Hide file tree
Showing 27 changed files with 182 additions and 103 deletions.
2 changes: 1 addition & 1 deletion community-build/community-projects/scodec
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/core/ConstraintHandling.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import NameKinds.AvoidNameKind
import util.SimpleIdentitySet
import NullOpsDecorator.stripNull

import scala.annotation.binaryAPI
import scala.annotation.{binaryAPI, binaryAPIAccessor}

/** Methods for adding constraints and solving them.
*
Expand Down Expand Up @@ -112,7 +112,8 @@ 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`.
*/
@binaryAPI private var myTrustBounds = true
@binaryAPIAccessor
private var myTrustBounds = true

inline def withUntrustedBounds(op: => Type): Type =
val saved = myTrustBounds
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Contexts.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import classfile.ReusableDataReader
import StdNames.nme
import compiletime.uninitialized

import scala.annotation.binaryAPI
import scala.annotation.{binaryAPI, binaryAPIAccessor}
import scala.annotation.internal.sharable

import DenotTransformers.DenotTransformer
Expand Down Expand Up @@ -800,7 +800,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
@binaryAPIAccessor
private def comparer(using Context): TypeComparer =
util.Stats.record("comparing")
val base = ctx.base
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,7 @@ class Definitions {
@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 BinaryAPIAccessorAnnot: ClassSymbol = requiredClass("scala.annotation.binaryAPIAccessor")

@tu lazy val JavaRepeatableAnnot: ClassSymbol = requiredClass("java.lang.annotation.Repeatable")

Expand Down
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1042,9 +1042,13 @@ object SymDenotations {
def isBinaryAPI(using Context): Boolean =
isTerm && (
hasAnnotation(defn.BinaryAPIAnnot) ||
allOverriddenSymbols.exists(_.hasAnnotation(defn.BinaryAPIAnnot))
allOverriddenSymbols.exists(sym => sym.hasAnnotation(defn.BinaryAPIAnnot))
)

/** Is this a member that will have an accessor in the generated binary */
def isBinaryAPIAccessor(using Context): Boolean =
isTerm && hasAnnotation(defn.BinaryAPIAccessorAnnot)

/** ()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.
Expand Down
7 changes: 4 additions & 3 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import reporting.trace
import annotation.constructorOnly
import cc.{CapturingType, derivedCapturingType, CaptureSet, stripCapturing, isBoxedCapturing, boxed, boxedUnlessFun, boxedIfTypeParam, isAlwaysPure}

import scala.annotation.binaryAPI
import scala.annotation.{binaryAPI, binaryAPIAccessor}

/** Provides methods to compare types.
*/
Expand Down Expand Up @@ -158,7 +158,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
private [this] var leftRoot: Type | Null = null

/** Are we forbidden from recording GADT constraints? */
@binaryAPI private var frozenGadt = false
@binaryAPIAccessor
private var frozenGadt = false
private inline def inFrozenGadt[T](inline op: T): T =
inFrozenGadtIf(true)(op)

Expand Down Expand Up @@ -189,7 +190,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
inFrozenGadtIf(true)(inFrozenConstraint(op))

extension (sym: Symbol)
@binaryAPI private inline def onGadtBounds(inline op: TypeBounds => Boolean): Boolean =
@binaryAPIAccessor private inline def onGadtBounds(inline op: TypeBounds => Boolean): Boolean =
val bounds = gadtBounds(sym)
bounds != null && op(bounds)

Expand Down
14 changes: 7 additions & 7 deletions compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ object PrepareInlineable {

def accessorNameOf(accessed: Symbol, site: Symbol)(using Context): TermName =
val accName = InlineAccessorName(accessed.name.asTermName)
if site.isExtensibleClass || accessed.isBinaryAPI then accName.expandedName(site)
if site.isExtensibleClass || accessed.isBinaryAPIAccessor then accName.expandedName(site)
else accName

/** A definition needs an accessor if it is private, protected, or qualified private
Expand All @@ -75,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.isBinaryAPI &&
!(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
!sym.isInlineMethod &&
(Inlines.inInlineMethod || StagingLevel.level > 0)
Expand Down Expand Up @@ -111,13 +111,14 @@ object PrepareInlineable {

protected def unstableAccessorWarning(accessor: Symbol, accessed: Symbol, srcPos: SrcPos)(using Context): Unit =
val accessorDefTree = accessorDef(accessor, accessed)
val annot = if accessed.is(Private) then "@binaryAPIAccessor" else "@binaryAPI"
val solution =
if accessed.is(Private) then
s"Annotate ${accessed.name} with `@binaryAPI` to generate a stable accessor."
s"Annotate ${accessed.name} with `$annot` to generate a stable accessor."
else
s"Annotate ${accessed.name} with `@binaryAPI` to make it accessible."
s"Annotate ${accessed.name} with `$annot` to make it accessible."
val binaryCompat =
s"""Adding @binaryAPI may break binary compatibility if a previous version of this
s"""Adding $annot 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}
Expand All @@ -140,7 +141,7 @@ object PrepareInlineable {
}
else
val accessorTree = useAccessor(tree)
if !tree.symbol.isBinaryAPI && tree.symbol != accessorTree.symbol then
if !tree.symbol.isBinaryAPI && !tree.symbol.isBinaryAPIAccessor && tree.symbol != accessorTree.symbol then
unstableAccessorWarning(accessorTree.symbol, tree.symbol, tree.srcPos)
accessorTree
case _ =>
Expand Down Expand Up @@ -252,7 +253,6 @@ object PrepareInlineable {

/** Create an inline accessor for this definition. */
def makePrivateBinaryAPIAccessor(sym: Symbol)(using Context): Unit =
assert(sym.is(Private))
if !sym.is(Accessor) then
val ref = tpd.ref(sym).asInstanceOf[RefTree]
val accessor = InsertPrivateBinaryAPIAccessors.useAccessor(ref)
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/parsing/Scanners.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import config.Feature.{migrateTo3, fewerBracesEnabled}
import config.SourceVersion.`3.0`
import reporting.{NoProfile, Profile, Message}

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor

import java.util.Objects

Expand Down Expand Up @@ -1598,7 +1598,8 @@ object Scanners {
protected def coversIndent(w: IndentWidth): Boolean =
knownWidth != null && w == indentWidth

@binaryAPI private var myCommasExpected: Boolean = false
@binaryAPIAccessor
private var myCommasExpected: Boolean = false

inline def withCommasExpected[T](inline op: => T): T =
val saved = myCommasExpected
Expand Down
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import Decorators.{em, toMessage}
import util.SourceFile
import Utility._

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor

// 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.
Expand Down Expand Up @@ -52,7 +52,7 @@ object MarkupParsers {
override def getMessage: String = "input ended while parsing XML"
}

class MarkupParser(@binaryAPI parser: Parser, final val preserveWS: Boolean)(using @binaryAPI c: Context) extends MarkupParserCommon {
class MarkupParser(@binaryAPIAccessor parser: Parser, final val preserveWS: Boolean)(using @binaryAPIAccessor c: Context) extends MarkupParserCommon {

import Tokens.{ LBRACE, RBRACE }

Expand Down Expand Up @@ -93,8 +93,8 @@ object MarkupParsers {
var xEmbeddedBlock: Boolean = false

private var debugLastStartElement = List.empty[(Int, String)]
@binaryAPI private def debugLastPos = debugLastStartElement.head._1
@binaryAPI private def debugLastElem = debugLastStartElement.head._2
@binaryAPIAccessor private def debugLastPos = debugLastStartElement.head._1
@binaryAPIAccessor private def debugLastElem = debugLastStartElement.head._2

private def errorBraces() = {
reportSyntaxError("in XML content, please use '}}' to express '}'")
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +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
import scala.annotation.binaryAPIAccessor

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
@binaryAPI private var myCtx: Context = super.printerContext
@binaryAPIAccessor private var myCtx: Context = super.printerContext
private var printPos = ctx.settings.YprintPos.value
private val printLines = ctx.settings.printLines.value

Expand Down
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/reporting/trace.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import core.*, Contexts.*, Decorators.*
import config.*
import printing.Formatting.*

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor
import scala.compiletime.*

/** Exposes the {{{ trace("question") { op } }}} syntax.
Expand Down Expand Up @@ -77,9 +77,11 @@ trait TraceSyntax:
inline def apply[T](inline question: String)(inline op: T)(using Context): T =
apply[T](question, false)(op)

@binaryAPI private val alwaysToString = (x: Any) => String.valueOf(x)
@binaryAPIAccessor
private val alwaysToString = (x: Any) => String.valueOf(x)

@binaryAPI private def doTrace[T](question: => String,
@binaryAPIAccessor
private def doTrace[T](question: => String,
printer: Printers.Printer = Printers.default,
showOp: T => String)
(op: => T)(using Context): T =
Expand Down
4 changes: 3 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,14 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
++ sym.annotations)
else
val binaryAPIAnnotOpt = sym.getAnnotation(defn.BinaryAPIAnnot)
val binaryAPIWithPrivateAccessorAnnotOpt = sym.getAnnotation(defn.BinaryAPIAccessorAnnot)
if sym.is(Param) then
sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
else if sym.is(ParamAccessor) then
// FIXME: copyAndKeepAnnotationsCarrying is dropping defn.BinaryAPIAnnot
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot, defn.BinaryAPIAnnot))
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot, defn.BinaryAPIAnnot, defn.BinaryAPIAccessorAnnot))
for binaryAPIAnnot <- binaryAPIAnnotOpt do sym.addAnnotation(binaryAPIAnnot)
for binaryAPIWithPrivateAccessorAnnot <- binaryAPIWithPrivateAccessorAnnotOpt do sym.addAnnotation(binaryAPIWithPrivateAccessorAnnot) // TODO is this one necessary?
else
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
if sym.isScala2Macro && !ctx.settings.XignoreScala2Macros.value then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package dotty.tools.dotc
package transform
package sjs

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor
import scala.collection.mutable

import ast.tpd
Expand Down Expand Up @@ -1123,7 +1123,7 @@ object PrepJSInterop {
val name: String = "prepjsinterop"
val description: String = "additional checks and transformations for Scala.js"

private final class OwnerKind private (@binaryAPI private val baseKinds: Int) extends AnyVal {
private final class OwnerKind private (@binaryAPIAccessor private val baseKinds: Int) extends AnyVal {

inline def isBaseKind: Boolean =
Integer.lowestOneBit(baseKinds) == baseKinds && baseKinds != 0 // exactly 1 bit on
Expand Down
5 changes: 5 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,11 @@ object Checking {
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) then fail(em"@binaryAPI cannot be used on private definitions.\n\nCould the definition `private[${sym.owner.name}]` or `protected` instead or use `@binaryAPIAccessor` instead of `@binaryAPI`.")
if sym.hasAnnotation(defn.BinaryAPIAccessorAnnot) then
if sym.is(Enum) then fail(em"@binaryAPIAccessor cannot be used on enum definitions.")
else if sym.isType && !sym.is(Module) && !(sym.is(Given) || sym.companionModule.is(Given)) then fail(em"@binaryAPIAccessor cannot be used on ${sym.showKind} definitions")
else if !sym.owner.isClass && !(sym.is(Param) && sym.owner.isConstructor) then fail(em"@binaryAPIAccessor cannot be used on local definitions.")
if (sym.hasAnnotation(defn.NativeAnnot)) {
if (!sym.is(Deferred))
fail(NativeMembersMayNotHaveImplementation(sym))
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import util.Property
import collection.mutable
import Trees._

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor

/** A class that handles argument lifting. Argument lifting is needed in the following
* scenarios:
Expand Down Expand Up @@ -165,7 +165,7 @@ object LiftComplex extends LiftComplex
object LiftCoverage extends LiftImpure {

// Property indicating whether we're currently lifting the arguments of an application
@binaryAPI private val LiftingArgs = new Property.Key[Boolean]
@binaryAPIAccessor private val LiftingArgs = new Property.Key[Boolean]

private inline def liftingArgs(using Context): Boolean =
ctx.property(LiftingArgs).contains(true)
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import dotty.tools.dotc.util.Spans._
import dotty.tools.dotc.util.Stats.record
import dotty.tools.dotc.reporting.IllegalVariableInPatternAlternative

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor
import scala.collection.mutable


Expand Down Expand Up @@ -212,7 +212,7 @@ trait QuotesAndSplices {
})

object splitter extends tpd.TreeMap {
@binaryAPI private var variance: Int = 1
@binaryAPIAccessor private var variance: Int = 1

inline private def atVariance[T](v: Int)(op: => T): T = {
val saved = variance
Expand Down
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2356,7 +2356,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
}
val vdef1 = assignType(cpy.ValDef(vdef)(name, tpt1, rhs1), sym)
postProcessInfo(sym)
binaryAPI(sym)
binaryAPIAccessors(sym)
vdef1.setDefTree
}

Expand Down Expand Up @@ -2460,7 +2460,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
val ddef2 = assignType(cpy.DefDef(ddef)(name, paramss1, tpt1, rhs1), sym)

postProcessInfo(sym)
binaryAPI(sym)
binaryAPIAccessors(sym)
ddef2.setDefTree
//todo: make sure dependent method types do not depend on implicits or by-name params
}
Expand All @@ -2475,8 +2475,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
sym.setFlag(Erased)

/** Generate inline accessors for definitions annotated with @inlineAccessible */
def binaryAPI(sym: Symbol)(using Context): Unit =
if !ctx.isAfterTyper && !sym.is(Param) && sym.is(Private) && sym.hasAnnotation(defn.BinaryAPIAnnot) then
def binaryAPIAccessors(sym: Symbol)(using Context): Unit =
if !ctx.isAfterTyper && !sym.is(Param) && sym.hasAnnotation(defn.BinaryAPIAccessorAnnot) then
PrepareInlineable.makePrivateBinaryAPIAccessor(sym)

def typedTypeDef(tdef: untpd.TypeDef, sym: Symbol)(using Context): Tree = {
Expand Down
10 changes: 6 additions & 4 deletions compiler/src/dotty/tools/dotc/util/ReusableInstance.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package dotty.tools.dotc.util
import scala.collection.mutable.ArrayBuffer
import scala.util.chaining._

import scala.annotation.binaryAPI
import scala.annotation.binaryAPIAccessor

/** A wrapper for a list of cached instances of a type `T`.
* The wrapper is recursion-reentrant: several instances are kept, so
Expand All @@ -16,9 +16,11 @@ import scala.annotation.binaryAPI
*
* Ported from scala.reflect.internal.util.ReusableInstance
*/
final class ReusableInstance[T <: AnyRef] private (@binaryAPI make: => T) {
@binaryAPI private[this] val cache = new ArrayBuffer[T](ReusableInstance.InitialSize).tap(_.addOne(make))
@binaryAPI private[this] var taken = 0
final class ReusableInstance[T <: AnyRef] private (@binaryAPIAccessor make: => T) {
@binaryAPIAccessor
private[this] val cache = new ArrayBuffer[T](ReusableInstance.InitialSize).tap(_.addOne(make))
@binaryAPIAccessor
private[this] var taken = 0

inline def withInstance[R](action: T => R): R ={
if (taken == cache.size)
Expand Down
Loading

0 comments on commit d4472ae

Please sign in to comment.