Skip to content

Commit a077c15

Browse files
committed
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`.
1 parent 8046a8b commit a077c15

31 files changed

+1196
-16
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ class Compiler {
7373
new RefChecks) :: // Various checks mostly related to abstract members and overriding
7474
List(new semanticdb.ExtractSemanticDB.AppendDiagnostics) :: // Attach warnings to extracted SemanticDB and write to .semanticdb file
7575
List(new init.Checker) :: // Check initialization of objects
76-
List(new ProtectedAccessors, // Add accessors for protected members
76+
List(new PublicInBinary, // Makes @publicInBinary definitions public
77+
new ProtectedAccessors, // Add accessors for protected members
7778
new ExtensionMethods, // Expand methods of value classes with extension methods
7879
new UncacheGivenAliases, // Avoid caching RHS of simple parameterless given aliases
7980
new ElimByName, // Map by-name parameters to functions

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ private sealed trait WarningSettings:
165165
val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
166166
val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
167167
val WimplausiblePatterns = BooleanSetting("-Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
168+
val WunstableInlineAccessors = BooleanSetting("-WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.")
168169
val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
169170
name = "-Wunused",
170171
helpArg = "warning",

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,6 +1056,7 @@ class Definitions {
10561056
@tu lazy val RequiresCapabilityAnnot: ClassSymbol = requiredClass("scala.annotation.internal.requiresCapability")
10571057
@tu lazy val RetainsAnnot: ClassSymbol = requiredClass("scala.annotation.retains")
10581058
@tu lazy val RetainsByNameAnnot: ClassSymbol = requiredClass("scala.annotation.retainsByName")
1059+
@tu lazy val PublicInBinaryAnnot: ClassSymbol = requiredClass("scala.annotation.publicInBinary")
10591060

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

@@ -1067,6 +1068,8 @@ class Definitions {
10671068
// A list of meta-annotations that are relevant for fields and accessors
10681069
@tu lazy val NonBeanMetaAnnots: Set[Symbol] =
10691070
Set(FieldMetaAnnot, GetterMetaAnnot, ParamMetaAnnot, SetterMetaAnnot, CompanionClassMetaAnnot, CompanionMethodMetaAnnot)
1071+
@tu lazy val NonBeanParamAccessorAnnots: Set[Symbol] =
1072+
Set(PublicInBinaryAnnot)
10701073
@tu lazy val MetaAnnots: Set[Symbol] =
10711074
NonBeanMetaAnnots + BeanGetterMetaAnnot + BeanSetterMetaAnnot
10721075

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,13 @@ object SymDenotations {
10311031
isOneOf(EffectivelyErased)
10321032
|| is(Inline) && !isRetainedInline && !hasAnnotation(defn.ScalaStaticAnnot)
10331033

1034+
/** Is this a member that will become public in the generated binary */
1035+
def hasPublicInBinary(using Context): Boolean =
1036+
isTerm && (
1037+
hasAnnotation(defn.PublicInBinaryAnnot) ||
1038+
allOverriddenSymbols.exists(sym => sym.hasAnnotation(defn.PublicInBinaryAnnot))
1039+
)
1040+
10341041
/** ()T and => T types should be treated as equivalent for this symbol.
10351042
* Note: For the moment, we treat Scala-2 compiled symbols as loose matching,
10361043
* because the Scala library does not always follow the right conventions.

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import transform.SymUtils.*
2323
import config.Printers.inlining
2424
import util.Property
2525
import staging.StagingLevel
26+
import dotty.tools.dotc.reporting.Message
27+
import dotty.tools.dotc.util.SrcPos
2628

2729
object PrepareInlineable {
2830
import tpd._
@@ -72,6 +74,7 @@ object PrepareInlineable {
7274
sym.isTerm &&
7375
(sym.isOneOf(AccessFlags) || sym.privateWithin.exists) &&
7476
!sym.isContainedIn(inlineSym) &&
77+
!sym.hasPublicInBinary &&
7578
!(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
7679
!sym.isInlineMethod &&
7780
(Inlines.inInlineMethod || StagingLevel.level > 0)
@@ -87,6 +90,11 @@ object PrepareInlineable {
8790

8891
override def transform(tree: Tree)(using Context): Tree =
8992
postTransform(super.transform(preTransform(tree)))
93+
94+
protected def checkUnstableAccessor(accessedTree: Tree, accessor: Symbol)(using Context): Unit =
95+
if ctx.settings.WunstableInlineAccessors.value then
96+
val accessorTree = accessorDef(accessor, accessedTree.symbol)
97+
report.warning(reporting.UnstableInlineAccessor(accessedTree.symbol, accessorTree), accessedTree)
9098
}
9199

92100
/** Direct approach: place the accessor with the accessed symbol. This has the
@@ -101,7 +109,11 @@ object PrepareInlineable {
101109
report.error("Implementation restriction: cannot use private constructors in inline methods", tree.srcPos)
102110
tree // TODO: create a proper accessor for the private constructor
103111
}
104-
else useAccessor(tree)
112+
else
113+
val accessor = useAccessor(tree)
114+
if tree != accessor then
115+
checkUnstableAccessor(tree, accessor.symbol)
116+
accessor
105117
case _ =>
106118
tree
107119
}
@@ -180,6 +192,8 @@ object PrepareInlineable {
180192
accessorInfo = abstractQualType(addQualType(dealiasMap(accessedType))),
181193
accessed = accessed)
182194

195+
checkUnstableAccessor(tree, accessor)
196+
183197
val (leadingTypeArgs, otherArgss) = splitArgs(argss)
184198
val argss1 = joinArgs(
185199
localRefs.map(TypeTree(_)) ++ leadingTypeArgs, // TODO: pass type parameters in two sections?

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,19 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
123123
else if homogenizedView then isEmptyPrefix(sym) // drop <root> and anonymous classes, but not scala, Predef.
124124
else if sym.isPackageObject then isOmittablePrefix(sym.owner)
125125
else isOmittablePrefix(sym)
126+
def isSkippedPackageObject(sym: Symbol) =
127+
sym.isPackageObject && !homogenizedView && !printDebug
126128

127129
tp.prefix match {
128-
case thisType: ThisType if isOmittable(thisType.cls) =>
129-
""
130-
case termRef @ TermRef(pre, _) =>
130+
case thisType: ThisType =>
131+
val sym = thisType.cls
132+
if isSkippedPackageObject(sym) then toTextPrefixOf(sym.typeRef)
133+
else if isOmittable(sym) then ""
134+
else super.toTextPrefixOf(tp)
135+
case termRef: TermRef =>
131136
val sym = termRef.symbol
132-
if sym.isPackageObject && !homogenizedView && !printDebug then toTextPrefixOf(termRef)
133-
else if (isOmittable(sym)) ""
137+
if isSkippedPackageObject(sym) then toTextPrefixOf(termRef)
138+
else if isOmittable(sym) then ""
134139
else super.toTextPrefixOf(tp)
135140
case _ => super.toTextPrefixOf(tp)
136141
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
204204
case VarArgsParamCannotBeGivenID // errorNumber: 188
205205
case ExtractorNotFoundID // errorNumber: 189
206206
case PureUnitExpressionID // errorNumber: 190
207+
case UnstableInlineAccessorID // errorNumber: 191
207208

208209
def errorNumber = ordinal - 1
209210

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3085,3 +3085,40 @@ class ImplausiblePatternWarning(pat: tpd.Tree, selType: Type)(using Context)
30853085
|$pat could match selector of type $selType
30863086
|only if there is an `equals` method identifying elements of the two types."""
30873087
def explain(using Context) = ""
3088+
3089+
class UnstableInlineAccessor(accessed: Symbol, accessorTree: tpd.Tree)(using Context)
3090+
extends Message(UnstableInlineAccessorID) {
3091+
def kind = MessageKind.Compatibility
3092+
3093+
def msg(using Context) =
3094+
i"""Unstable inline accessor ${accessor.name} was generated in $where."""
3095+
3096+
def explain(using Context) =
3097+
i"""Access to non-public $accessed causes the automatic generation of an accessor.
3098+
|This accessor is not stable, its name may change or it may disappear
3099+
|if not needed in a future version.
3100+
|
3101+
|To make sure that the inlined code is binary compatible you must make sure that
3102+
|$accessed is public in the binary API.
3103+
| * Option 1: Annotate $accessed with @publicInBinary
3104+
| * Option 2: Make $accessed public
3105+
|
3106+
|This change may break binary compatibility if a previous version of this
3107+
|library was compiled with generated accessors. Binary compatibility should
3108+
|be checked using MiMa. If binary compatibility is broken, you should add the
3109+
|old accessor explicitly in the source code. The following code should be
3110+
|added to $where:
3111+
| @publicInBinary private[$within] ${accessorTree.show}
3112+
|"""
3113+
3114+
private def accessor = accessorTree.symbol
3115+
3116+
private def where =
3117+
if accessor.owner.name.isPackageObjectName then s"package ${within}"
3118+
else if accessor.owner.is(Module) then s"object $within"
3119+
else s"class $within"
3120+
3121+
private def within =
3122+
if accessor.owner.name.isPackageObjectName then accessor.owner.owner.name.stripModuleClassSuffix
3123+
else accessor.owner.name.stripModuleClassSuffix
3124+
}

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,11 @@ abstract class AccessProxies {
3232
/** The accessor definitions that need to be added to class `cls` */
3333
private def accessorDefs(cls: Symbol)(using Context): Iterator[DefDef] =
3434
for accessor <- cls.info.decls.iterator; accessed <- accessedBy.get(accessor) yield
35-
DefDef(accessor.asTerm, prefss => {
35+
accessorDef(accessor, accessed)
36+
37+
protected def accessorDef(accessor: Symbol, accessed: Symbol)(using Context): DefDef =
38+
DefDef(accessor.asTerm,
39+
prefss => {
3640
def numTypeParams = accessed.info match {
3741
case info: PolyType => info.paramNames.length
3842
case _ => 0
@@ -42,7 +46,7 @@ abstract class AccessProxies {
4246
if (passReceiverAsArg(accessor.name))
4347
(argss.head.head.select(accessed), targs.takeRight(numTypeParams), argss.tail)
4448
else
45-
(if (accessed.isStatic) ref(accessed) else ref(TermRef(cls.thisType, accessed)),
49+
(if (accessed.isStatic) ref(accessed) else ref(TermRef(accessor.owner.thisType, accessed)),
4650
targs, argss)
4751
val rhs =
4852
if (accessor.name.isSetterName &&
@@ -54,7 +58,8 @@ abstract class AccessProxies {
5458
.appliedToArgss(forwardedArgss)
5559
.etaExpandCFT(using ctx.withOwner(accessor))
5660
rhs.withSpan(accessed.span)
57-
})
61+
}
62+
)
5863

5964
/** Add all needed accessors to the `body` of class `cls` */
6065
def addAccessorDefs(cls: Symbol, body: List[Tree])(using Context): List[Tree] = {
@@ -149,7 +154,7 @@ abstract class AccessProxies {
149154
def accessorIfNeeded(tree: Tree)(using Context): Tree = tree match {
150155
case tree: RefTree if needsAccessor(tree.symbol) =>
151156
if (tree.symbol.isConstructor) {
152-
report.error("Implementation restriction: cannot use private constructors in inlineable methods", tree.srcPos)
157+
report.error("Cannot use private constructors in inline methods. You can use @publicInBinary to make constructor accessible in inline methods.", tree.srcPos)
153158
tree // TODO: create a proper accessor for the private constructor
154159
}
155160
else useAccessor(tree)

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,10 @@ class PostTyper extends MacroTransform with InfoTransformer { thisPhase =>
172172
if sym.is(Param) then
173173
sym.keepAnnotationsCarrying(thisPhase, Set(defn.ParamMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
174174
else if sym.is(ParamAccessor) then
175+
// @publicInBinary is not a meta-annotation and therefore not kept by `keepAnnotationsCarrying`
176+
val publicInBinaryAnnotOpt = sym.getAnnotation(defn.PublicInBinaryAnnot)
175177
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot))
178+
for publicInBinaryAnnot <- publicInBinaryAnnotOpt do sym.addAnnotation(publicInBinaryAnnot)
176179
else
177180
sym.keepAnnotationsCarrying(thisPhase, Set(defn.GetterMetaAnnot, defn.FieldMetaAnnot), orNoneOf = defn.NonBeanMetaAnnots)
178181
if sym.isScala2Macro && !ctx.settings.XignoreScala2Macros.value then
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package dotty.tools.dotc.transform
2+
3+
import dotty.tools.dotc.core.Contexts.*
4+
import dotty.tools.dotc.core.DenotTransformers.SymTransformer
5+
import dotty.tools.dotc.core.Flags.*
6+
import dotty.tools.dotc.core.Symbols.NoSymbol
7+
import dotty.tools.dotc.core.SymDenotations.SymDenotation
8+
import dotty.tools.dotc.transform.MegaPhase.MiniPhase
9+
import dotty.tools.dotc.typer.RefChecks
10+
11+
/** Makes @publicInBinary definitions public.
12+
*
13+
* This makes it possible to elide the generations of some unnecessary accessors.
14+
*/
15+
class PublicInBinary extends MiniPhase with SymTransformer:
16+
17+
override def runsAfterGroupsOf: Set[String] = Set(RefChecks.name)
18+
19+
override def phaseName: String = PublicInBinary.name
20+
override def description: String = PublicInBinary.description
21+
22+
def transformSym(d: SymDenotation)(using Context): SymDenotation = {
23+
if d.hasPublicInBinary then
24+
d.resetFlag(Protected)
25+
d.setPrivateWithin(NoSymbol)
26+
if d.is(Module) then
27+
val moduleClass = d.moduleClass
28+
moduleClass.resetFlag(Protected)
29+
moduleClass.setPrivateWithin(NoSymbol)
30+
d
31+
}
32+
33+
object PublicInBinary:
34+
val name: String = "publicInBinary"
35+
val description: String = "makes @publicInBinary definitions public"

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,11 @@ object Checking {
543543
fail(em"Inline methods cannot be @tailrec")
544544
if sym.hasAnnotation(defn.TargetNameAnnot) && sym.isClass && sym.isTopLevelClass then
545545
fail(TargetNameOnTopLevelClass(sym))
546+
if sym.hasAnnotation(defn.PublicInBinaryAnnot) then
547+
if sym.is(Enum) then fail(em"@publicInBinary cannot be used on enum definitions")
548+
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")
549+
else if !sym.owner.isClass && !(sym.is(Param) && sym.owner.isConstructor) then fail(em"@publicInBinary cannot be used on local definitions")
550+
else if sym.is(Private) && !sym.privateWithin.exists && !sym.isConstructor then fail(em"@publicInBinary cannot be used on private definitions\n\nCould the definition `private[${sym.owner.name}]` or `protected` instead")
546551
if (sym.hasAnnotation(defn.NativeAnnot)) {
547552
if (!sym.is(Deferred))
548553
fail(NativeMembersMayNotHaveImplementation(sym))

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,9 @@ trait TypeAssigner {
107107
val tpe1 = accessibleType(tpe, superAccess)
108108
if tpe1.exists then tpe1
109109
else tpe match
110-
case tpe: NamedType => inaccessibleErrorType(tpe, superAccess, pos)
111-
case NoType => tpe
110+
case tpe: NamedType if !tpe.termSymbol.hasPublicInBinary =>
111+
inaccessibleErrorType(tpe, superAccess, pos)
112+
case _ => tpe
112113

113114
/** Return a potentially skolemized version of `qualTpe` to be used
114115
* as a prefix when selecting `name`.

0 commit comments

Comments
 (0)