Skip to content

Commit 725810c

Browse files
nicolasstuckinicolasstucki
committed
Part of the fix for scala#15413 Part of the fix for scala#16983
1 parent 847eccc commit 725810c

File tree

15 files changed

+223
-14
lines changed

15 files changed

+223
-14
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ class Compiler {
8989
new ExplicitOuter, // Add accessors to outer classes from nested ones.
9090
new ExplicitSelf, // Make references to non-trivial self types explicit as casts
9191
new StringInterpolatorOpt, // Optimizes raw and s and f string interpolators by rewriting them to string concatenations or formats
92-
new DropBreaks) :: // Optimize local Break throws by rewriting them
92+
new DropBreaks) :: // Optimize local Break throws by rewriting them
9393
List(new PruneErasedDefs, // Drop erased definitions from scopes and simplify erased expressions
9494
new UninitializedDefs, // Replaces `compiletime.uninitialized` by `_`
9595
new InlinePatterns, // Remove placeholders of inlined patterns
@@ -141,6 +141,7 @@ class Compiler {
141141
new sjs.JUnitBootstrappers, // Generate JUnit-specific bootstrapper classes for Scala.js (not enabled by default)
142142
new CollectEntryPoints, // Collect all entry points and save them in the context
143143
new CollectSuperCalls, // Find classes that are called with super
144+
new BinaryAPIAnnotations, // Makes @binaryAPI definitions public // TODO where should this phase be located?
144145
new RepeatableAnnotations) :: // Aggregate repeatable annotations
145146
Nil
146147

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,7 @@ class Definitions {
10471047
@tu lazy val RequiresCapabilityAnnot: ClassSymbol = requiredClass("scala.annotation.internal.requiresCapability")
10481048
@tu lazy val RetainsAnnot: ClassSymbol = requiredClass("scala.annotation.retains")
10491049
@tu lazy val RetainsByNameAnnot: ClassSymbol = requiredClass("scala.annotation.retainsByName")
1050+
@tu lazy val BinaryAPIAnnot: ClassSymbol = requiredClass("scala.annotation.binaryAPI")
10501051

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

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

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ object PrepareInlineable {
5353
/** A tree map which inserts accessors for non-public term members accessed from inlined code.
5454
*/
5555
abstract class MakeInlineableMap(val inlineSym: Symbol) extends TreeMap with Insert {
56+
57+
def useBinaryAPI: Boolean
58+
5659
def accessorNameOf(name: TermName, site: Symbol)(using Context): TermName =
5760
val accName = InlineAccessorName(name)
5861
if site.isExtensibleClass then accName.expandedName(site) else accName
@@ -70,6 +73,7 @@ object PrepareInlineable {
7073
def needsAccessor(sym: Symbol)(using Context): Boolean =
7174
sym.isTerm &&
7275
(sym.isOneOf(AccessFlags) || sym.privateWithin.exists) &&
76+
(!useBinaryAPI || !sym.hasAnnotation(defn.BinaryAPIAnnot)) &&
7377
!sym.isContainedIn(inlineSym) &&
7478
!(sym.isStableMember && sym.info.widenTermRefExpr.isInstanceOf[ConstantType]) &&
7579
!sym.isInlineMethod &&
@@ -100,7 +104,7 @@ object PrepareInlineable {
100104
* possible if the receiver is essentially this or an outer this, which is indicated
101105
* by the test that we can find a host for the accessor.
102106
*/
103-
class MakeInlineableDirect(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
107+
class MakeInlineableDirect(inlineSym: Symbol, val useBinaryAPI: Boolean) extends MakeInlineableMap(inlineSym) {
104108
def preTransform(tree: Tree)(using Context): Tree = tree match {
105109
case tree: RefTree if needsAccessor(tree.symbol) =>
106110
if (tree.symbol.isConstructor) {
@@ -124,13 +128,14 @@ object PrepareInlineable {
124128
* private[inlines] def next[U](y: U): (T, U) = (x, y)
125129
* }
126130
* class TestPassing {
127-
* inline def foo[A](x: A): (A, Int) = {
128-
* val c = new C[A](x)
129-
* c.next(1)
130-
* }
131-
* inline def bar[A](x: A): (A, String) = {
132-
* val c = new C[A](x)
133-
* c.next("")
131+
* inline def foo[A](x: A): (A, Int) = {
132+
* val c = new C[A](x)
133+
* c.next(1)
134+
* }
135+
* inline def bar[A](x: A): (A, String) = {
136+
* val c = new C[A](x)
137+
* c.next("")
138+
* }
134139
* }
135140
*
136141
* `C` could be compiled separately, so we cannot place the inline accessor in it.
@@ -143,7 +148,7 @@ object PrepareInlineable {
143148
* Since different calls might have different receiver types, we need to generate one
144149
* such accessor per call, so they need to have unique names.
145150
*/
146-
class MakeInlineablePassing(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
151+
class MakeInlineablePassing(inlineSym: Symbol, val useBinaryAPI: Boolean) extends MakeInlineableMap(inlineSym) {
147152

148153
def preTransform(tree: Tree)(using Context): Tree = tree match {
149154
case _: Apply | _: TypeApply | _: RefTree
@@ -153,7 +158,7 @@ object PrepareInlineable {
153158
val qual = qualifier(refPart)
154159
inlining.println(i"adding receiver passing inline accessor for $tree/$refPart -> (${qual.tpe}, $refPart: ${refPart.getClass}, $argss%, %")
155160

156-
// Need to dealias in order to cagtch all possible references to abstracted over types in
161+
// Need to dealias in order to catch all possible references to abstracted over types in
157162
// substitutions
158163
val dealiasMap = new TypeMap {
159164
def apply(t: Type) = mapOver(t.dealias)
@@ -226,8 +231,13 @@ object PrepareInlineable {
226231
// so no accessors are needed for them.
227232
tree
228233
else
229-
new MakeInlineablePassing(inlineSym).transform(
230-
new MakeInlineableDirect(inlineSym).transform(tree))
234+
// Make sure the old accessors are generated for binary compatibility
235+
new MakeInlineablePassing(inlineSym, useBinaryAPI = false).transform(
236+
new MakeInlineableDirect(inlineSym, useBinaryAPI = false).transform(tree))
237+
238+
// TODO: warn if MakeInlineablePassing or MakeInlineableDirect generate accessors when useBinaryAPI in enabled
239+
new MakeInlineablePassing(inlineSym, useBinaryAPI = true).transform(
240+
new MakeInlineableDirect(inlineSym, useBinaryAPI = true).transform(tree))
231241
}
232242
}
233243

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package dotty.tools.dotc
2+
package transform
3+
4+
import core._
5+
import ast.tpd._
6+
import Contexts._
7+
import MegaPhase._
8+
import Annotations._
9+
import Symbols.defn
10+
import Constants._
11+
import Types._
12+
import Symbols.*
13+
import Decorators._
14+
import Flags._
15+
import DenotTransformers.SymTransformer
16+
import SymDenotations.SymDenotation
17+
18+
/** Makes @binaryAPI definitions public */
19+
class BinaryAPIAnnotations extends MiniPhase with SymTransformer:
20+
21+
override def phaseName: String = BinaryAPIAnnotations.name
22+
23+
override def description: String = BinaryAPIAnnotations.description
24+
25+
def transformSym(d: SymDenotation)(using Context): SymDenotation =
26+
if d.hasAnnotation(defn.BinaryAPIAnnot) then
27+
d.resetFlag(Protected)
28+
d.setPrivateWithin(NoSymbol)
29+
d
30+
31+
end BinaryAPIAnnotations
32+
33+
object BinaryAPIAnnotations:
34+
val name: String = "binaryAPIAnnotations"
35+
val description: String = "makes @binaryAPI definitions public"

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,10 @@ object Checking {
526526
fail(em"Inline methods cannot be @tailrec")
527527
if sym.hasAnnotation(defn.TargetNameAnnot) && sym.isClass && sym.isTopLevelClass then
528528
fail(TargetNameOnTopLevelClass(sym))
529+
if sym.hasAnnotation(defn.BinaryAPIAnnot) then
530+
if sym.isType && !(sym.is(Given) || sym.companionModule.is(Given)) then fail(em"@binaryAPI cannot be used on ${sym.showKind} definitions")
531+
else if sym.isLocal then fail(em"@binaryAPI cannot be used on local definitions.")
532+
else if sym.is(Private) then fail(em"@binaryAPI cannot be used on private definitions.\n\nCould use private[${sym.owner.name}] or protected instead.")
529533
if (sym.hasAnnotation(defn.NativeAnnot)) {
530534
if (!sym.is(Deferred))
531535
fail(NativeMembersMayNotHaveImplementation(sym))

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,9 @@ trait TypeAssigner {
9999
val tpe1 = accessibleType(tpe, superAccess)
100100
if tpe1.exists then tpe1
101101
else tpe match
102-
case tpe: NamedType => inaccessibleErrorType(tpe, superAccess, pos)
102+
case tpe: NamedType =>
103+
if tpe.termSymbol.hasAnnotation(defn.BinaryAPIAnnot) then tpe
104+
else inaccessibleErrorType(tpe, superAccess, pos)
103105
case NoType => tpe
104106

105107
/** Return a potentially skolemized version of `qualTpe` to be used

‎compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,62 @@ class DottyBytecodeTests extends DottyBytecodeTest {
16821682
assertSameCode(instructions, expected)
16831683
}
16841684
}
1685+
1686+
@Test
1687+
def i15413(): Unit = {
1688+
val code =
1689+
"""import scala.quoted.*
1690+
|import scala.annotation.binaryAPI
1691+
|class Macro:
1692+
| inline def foo = Macro.fooImpl
1693+
| def test = foo
1694+
|object Macro:
1695+
| @binaryAPI private[Macro] def fooImpl = {}
1696+
""".stripMargin
1697+
checkBCode(code) { dir =>
1698+
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
1699+
1700+
// For 3.0-3.3 compat
1701+
val macroClass = loadClassNode(dir.lookupName("Macro.class", directory = false).input, skipDebugInfo = false)
1702+
val oldAccessor = getMethod(macroClass, "Macro$$inline$fooImpl")
1703+
assert(oldAccessor.signature == "()V")
1704+
assert((oldAccessor.access & privateAccessors) == 0)
1705+
1706+
// Check that the @binaryAPI annotated method is called
1707+
val testMethod = getMethod(macroClass, "test")
1708+
val testInstructions = instructionsFromMethod(testMethod).filter(_.isInstanceOf[Invoke])
1709+
assertSameCode(testInstructions, List(
1710+
Invoke(INVOKEVIRTUAL, "Macro$", "fooImpl", "()V", false)))
1711+
}
1712+
}
1713+
1714+
@Test
1715+
def i15413b(): Unit = {
1716+
val code =
1717+
"""package foo
1718+
|import scala.annotation.binaryAPI
1719+
|class C:
1720+
| inline def baz = D.bazImpl
1721+
| def test = baz
1722+
|object D:
1723+
| @binaryAPI private[foo] def bazImpl = {}
1724+
""".stripMargin
1725+
checkBCode(code) { dir =>
1726+
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
1727+
1728+
// For 3.0-3.3 compat
1729+
val barClass = loadClassNode(dir.subdirectoryNamed("foo").lookupName("C.class", directory = false).input, skipDebugInfo = false)
1730+
val oldAccessor = getMethod(barClass, "inline$bazImpl$i1")
1731+
assert(oldAccessor.desc == "(Lfoo/D$;)V", oldAccessor.desc)
1732+
assert((oldAccessor.access & privateAccessors) == 0)
1733+
1734+
// Check that the @binaryAPI annotated method is called
1735+
val testMethod = getMethod(barClass, "test")
1736+
val testInstructions = instructionsFromMethod(testMethod).filter(_.isInstanceOf[Invoke])
1737+
assertSameCode(testInstructions, List(
1738+
Invoke(INVOKEVIRTUAL, "foo/D$", "bazImpl", "()V", false)))
1739+
}
1740+
}
16851741
}
16861742

16871743
object invocationReceiversTestCode {

‎library/src/scala/annotation/binaryAPI.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package scala.annotation
2+
3+
/** TODO */
4+
@experimental
5+
class binaryAPI extends annotation.StaticAnnotation

‎tests/neg/binaryAPI.scala

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package foo
2+
3+
import scala.annotation.binaryAPI
4+
5+
@binaryAPI type A // error
6+
@binaryAPI object B // error // TODO support?
7+
@binaryAPI class C: // error
8+
@binaryAPI private def f: Unit = // error
9+
@binaryAPI def g = () // error
10+
()

‎tests/pos-macros/i15413/Macro_1.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import scala.quoted.*
2+
import scala.annotation.binaryAPI
3+
4+
class Macro:
5+
inline def foo = ${ Macro.fooImpl }
6+
7+
object Macro:
8+
@binaryAPI private[Macro] def fooImpl(using Quotes) = '{}

‎tests/pos-macros/i15413/Test_2.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
def test =
2+
new Macro().foo

‎tests/pos-macros/i15413b/Macro_1.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package bar
2+
3+
import scala.quoted.*
4+
import scala.annotation.binaryAPI
5+
6+
inline def foo = ${ fooImpl }
7+
8+
@binaryAPI private[bar] def fooImpl(using Quotes) = '{}

‎tests/pos-macros/i15413b/Test_2.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
def test = bar.foo

‎tests/pos/binaryAPI.scala

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package foo
2+
3+
import scala.annotation.binaryAPI
4+
5+
class Foo(@binaryAPI param: Int): // FIXME
6+
@binaryAPI
7+
protected val protectedVal: Int = 2
8+
9+
@binaryAPI
10+
private[foo] def packagePrivateVal: Int = 2
11+
inline def foo: Int = param + protectedVal + packagePrivateVal
12+
13+
class Bar() extends Foo(3):
14+
override protected val protectedVal: Int = 2
15+
16+
override private[foo] def packagePrivateVal: Int = 2
17+
18+
inline def bar: Int = protectedVal + packagePrivateVal // FIXME not use the inline accessors as Foo.{protectedVal,packagePrivateVal} are @binaryAPI
19+
20+
class Baz() extends Foo(4):
21+
@binaryAPI // TODO warn? Not needed because Foo.protectedVal is already @binaryAPI
22+
override protected val protectedVal: Int = 2 // FIXME should remove protected
23+
24+
@binaryAPI
25+
override private[foo] def packagePrivateVal: Int = 2 // FIXME should remove private[foo]
26+
27+
inline def baz: Int = protectedVal + packagePrivateVal
28+
29+
30+
class Qux() extends Foo(5):
31+
inline def qux: Int = protectedVal + packagePrivateVal
32+
33+
def test =
34+
Foo(3).foo
35+
Bar().bar
36+
Baz().baz
37+
Qux().qux
38+
39+
@binaryAPI given Int = 1
40+
@binaryAPI given (using Double): Int = 1
41+
42+
trait A[T]:
43+
def f: T
44+
@binaryAPI given A[Int] with
45+
def f: Int = 1
46+
@binaryAPI given (using Double): A[Int] with
47+
def f: Int = 1
48+
49+
package inlines {
50+
// Case that needed to be converted with MakeInlineablePassing
51+
class C[T](x: T) {
52+
@binaryAPI private[inlines] def next[U](y: U): (T, U) = (x, y)
53+
}
54+
class TestPassing {
55+
inline def foo[A](x: A): (A, Int) = {
56+
val c = new C[A](x)
57+
c.next(1)
58+
}
59+
inline def bar[A](x: A): (A, String) = {
60+
val c = new C[A](x)
61+
c.next("")
62+
}
63+
}
64+
}

‎tests/run-custom-args/tasty-inspector/stdlibExperimentalDefinitions.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ val experimentalDefinitionInLibrary = Set(
3535
"scala.annotation.MainAnnotation$.Parameter",
3636
"scala.annotation.MainAnnotation$.ParameterAnnotation",
3737

38+
//// New feature: Explicit inline accessors
39+
"scala.annotation.binaryAPI",
3840

3941
//// New feature: prototype of new version of @main
4042
// This will never be stabilized. When it is ready it should replace the old @main annotation (requires scala.annotation.MainAnnotation).

0 commit comments

Comments
 (0)