Skip to content

Commit

Permalink
Add @inlineAccessible annotation
Browse files Browse the repository at this point in the history
Part of the fix for scala#15413
Part of the fix for scala#16983
  • Loading branch information
nicolasstucki committed Feb 22, 2023
1 parent 847eccc commit eaf0804
Show file tree
Hide file tree
Showing 13 changed files with 218 additions and 18 deletions.
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 @@ -1047,6 +1047,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 InlineAccessibleAnnot: ClassSymbol = requiredClass("scala.annotation.inlineAccessible")

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

Expand Down
53 changes: 43 additions & 10 deletions compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ object PrepareInlineable {
def makeInlineable(tree: Tree)(using Context): Tree =
ctx.property(InlineAccessorsKey).get.makeInlineable(tree)

def makeInlineAccessible(sym: Symbol)(using Context): Unit =
ctx.property(InlineAccessorsKey).get.makeInlineAccessible(sym)

def addAccessorDefs(cls: Symbol, body: List[Tree])(using Context): List[Tree] =
ctx.property(InlineAccessorsKey) match
case Some(inlineAccessors) => inlineAccessors.addAccessorDefs(cls, body)
Expand Down Expand Up @@ -95,6 +98,20 @@ object PrepareInlineable {
case _ => ctx
}

/** Inline accessible approach: use the accessors generated by @inlineAccessible */
class MakeInlineableAccessible(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
def preTransform(tree: Tree)(using Context): Tree = tree match {
case tree: RefTree if needsAccessor(tree.symbol) && tree.symbol.hasAnnotation(defn.InlineAccessibleAnnot) =>
val ref = tpd.ref(tree.symbol).asInstanceOf[RefTree]
val accessorHost = tree.symbol.owner
val accessor = useAccessor(ref, accessorHost).symbol
rewire(tree, accessor)
case _ =>
tree
}
override def ifNoHost(reference: RefTree)(using Context): Tree = reference
}

/** Direct approach: place the accessor with the accessed symbol. This has the
* advantage that we can re-use the receiver as is. But it is only
* possible if the receiver is essentially this or an outer this, which is indicated
Expand All @@ -103,11 +120,15 @@ object PrepareInlineable {
class MakeInlineableDirect(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
def preTransform(tree: Tree)(using Context): Tree = tree match {
case tree: RefTree if needsAccessor(tree.symbol) =>
if (tree.symbol.isConstructor) {
if tree.symbol.isConstructor then
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
// if !tree.symbol.hasAnnotation(defn.InlineAccessibleAnnot) then
// report.error(em"Inline access to private definition.\n\n${tree.symbol} should be annotated with @inlineAccessible.\nAlternatively, create a private @inlineAccessible alias to this definition.", tree.srcPos)
val nearestHost = AccessProxies.hostForAccessorOf(tree.symbol)
val host = if nearestHost.is(Package) then ctx.owner.topLevelClass else nearestHost
useAccessor(tree, host)
case _ =>
tree
}
Expand All @@ -125,12 +146,13 @@ object PrepareInlineable {
* }
* class TestPassing {
* inline def foo[A](x: A): (A, Int) = {
* val c = new C[A](x)
* c.next(1)
* }
* inline def bar[A](x: A): (A, String) = {
* val c = new C[A](x)
* c.next("")
* val c = new C[A](x)
* c.next(1)
* }
* inline def bar[A](x: A): (A, String) = {
* val c = new C[A](x)
* c.next("")
* }
* }
*
* `C` could be compiled separately, so we cannot place the inline accessor in it.
Expand All @@ -153,7 +175,7 @@ object PrepareInlineable {
val qual = qualifier(refPart)
inlining.println(i"adding receiver passing inline accessor for $tree/$refPart -> (${qual.tpe}, $refPart: ${refPart.getClass}, $argss%, %")

// Need to dealias in order to cagtch all possible references to abstracted over types in
// Need to dealias in order to catch all possible references to abstracted over types in
// substitutions
val dealiasMap = new TypeMap {
def apply(t: Type) = mapOver(t.dealias)
Expand Down Expand Up @@ -211,6 +233,11 @@ object PrepareInlineable {
}
}

/** Create an inline accessor for this definition. */
def makeInlineAccessible(sym: Symbol)(using Context): Unit =
val ref = tpd.ref(sym).asInstanceOf[RefTree]
MakeInlineableDirect(NoSymbol).useAccessor(ref, sym.owner)

/** Adds accessors for all non-public term members accessed
* from `tree`. Non-public type members are currently left as they are.
* This means that references to a private type will lead to typing failures
Expand All @@ -226,8 +253,14 @@ object PrepareInlineable {
// so no accessors are needed for them.
tree
else
// Make sure the old accessors are generated
new MakeInlineablePassing(inlineSym).transform(
new MakeInlineableDirect(inlineSym).transform(tree))

// Transform the tree adding all inline accessors
new MakeInlineablePassing(inlineSym).transform(
new MakeInlineableDirect(inlineSym).transform(
new MakeInlineableAccessible(inlineSym).transform(tree)))
}
}

Expand Down
13 changes: 5 additions & 8 deletions compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ abstract class AccessProxies {
}

/** Rewire reference to refer to `accessor` symbol */
private def rewire(reference: RefTree, accessor: Symbol)(using Context): Tree = {
protected def rewire(reference: RefTree, accessor: Symbol)(using Context): Tree = {
reference match {
case Select(qual, _) if qual.tpe.derivesFrom(accessor.owner) => qual.select(accessor)
case _ => ref(accessor)
Expand Down Expand Up @@ -127,15 +127,12 @@ abstract class AccessProxies {
/** Create an accessor unless one exists already, and replace the original
* access with a reference to the accessor.
*
* @param reference The original reference to the non-public symbol
* @param onLHS The reference is on the left-hand side of an assignment
* @param reference The original reference to the non-public symbol
* @param accessorClass The owner of the accessor
*/
def useAccessor(reference: RefTree)(using Context): Tree = {
def useAccessor(reference: RefTree, accessorClass: Symbol)(using Context): Tree = {
val accessed = reference.symbol.asTerm
var accessorClass = hostForAccessorOf(accessed: Symbol)
if (accessorClass.exists) {
if accessorClass.is(Package) then
accessorClass = ctx.owner.topLevelClass
val accessorName = accessorNameOf(accessed.name, accessorClass)
val accessorInfo =
accessed.info.ensureMethodic.asSeenFrom(accessorClass.thisType, accessed.owner)
Expand All @@ -152,7 +149,7 @@ abstract class AccessProxies {
report.error("Implementation restriction: cannot use private constructors in inlineable methods", tree.srcPos)
tree // TODO: create a proper accessor for the private constructor
}
else useAccessor(tree)
else useAccessor(tree, hostForAccessorOf(tree.symbol))
case _ =>
tree
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2347,6 +2347,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
}
val vdef1 = assignType(cpy.ValDef(vdef)(name, tpt1, rhs1), sym)
postProcessInfo(sym)
inlineAccessors(sym)
vdef1.setDefTree
}

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

postProcessInfo(sym)
inlineAccessors(sym)
ddef2.setDefTree
//todo: make sure dependent method types do not depend on implicits or by-name params
}
Expand All @@ -2463,6 +2465,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
if !sym.is(Module) && !sym.isConstructor && sym.info.finalResultType.isErasedClass then
sym.setFlag(Erased)

/** Generate inline accessors for definitions annotated with @inlineAccessible */
def inlineAccessors(sym: Symbol)(using Context): Unit =
if !ctx.isAfterTyper && !sym.is(Param) && sym.hasAnnotation(defn.InlineAccessibleAnnot) then
PrepareInlineable.makeInlineAccessible(sym)

def typedTypeDef(tdef: untpd.TypeDef, sym: Symbol)(using Context): Tree = {
val TypeDef(name, rhs) = tdef
completeAnnotations(tdef, sym)
Expand Down
68 changes: 68 additions & 0 deletions compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1682,6 +1682,74 @@ class DottyBytecodeTests extends DottyBytecodeTest {
assertSameCode(instructions, expected)
}
}

@Test
def i15413(): Unit = {
val code =
"""import scala.quoted.*
|import scala.annotation.inlineAccessible
|class Macro:
| inline def foo = Macro.fooImpl
| def test = foo
|object Macro:
| @inlineAccessible private def fooImpl = {}
""".stripMargin
checkBCode(code) { dir =>
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED

// For 3.0-3.3 compat
val macroClass = loadClassNode(dir.lookupName("Macro.class", directory = false).input, skipDebugInfo = false)
val oldAccessor = getMethod(macroClass, "Macro$$inline$fooImpl")
assert(oldAccessor.signature == "()V")
assert((oldAccessor.access & privateAccessors) == 0)

// For 3.4+
val moduleClass = loadClassNode(dir.lookupName("Macro$.class", directory = false).input, skipDebugInfo = false)
val newAccessor = getMethod(moduleClass, "inline$fooImpl")
assert(newAccessor.signature == "()V")
assert((newAccessor.access & privateAccessors) == 0)

// Check that the @inlineAccessible generated accessor is used
val testMethod = getMethod(macroClass, "test")
val testInstructions = instructionsFromMethod(testMethod).filter(_.isInstanceOf[Invoke])
assertSameCode(testInstructions, List(
Invoke(INVOKEVIRTUAL, "Macro$", "inline$fooImpl", "()V", false)))
}
}

@Test
def i15413b(): Unit = {
val code =
"""package foo
|import scala.annotation.inlineAccessible
|class C:
| inline def baz = D.bazImpl
| def test = baz
|object D:
| @inlineAccessible private[foo] def bazImpl = {}
""".stripMargin
checkBCode(code) { dir =>
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED

// For 3.0-3.3 compat
val barClass = loadClassNode(dir.subdirectoryNamed("foo").lookupName("C.class", directory = false).input, skipDebugInfo = false)
val oldAccessor = getMethod(barClass, "inline$bazImpl$i1")
assert(oldAccessor.desc == "(Lfoo/D$;)V", oldAccessor.desc)
assert((oldAccessor.access & privateAccessors) == 0)

// For 3.4+
val dModuleClass = loadClassNode(dir.subdirectoryNamed("foo").lookupName("D$.class", directory = false).input, skipDebugInfo = false)
val newAccessor = getMethod(dModuleClass, "inline$bazImpl")
assert(newAccessor.signature == "()V", newAccessor.signature)
assert((newAccessor.access & privateAccessors) == 0)

// Check that the @inlineAccessible generated accessor is used
val testMethod = getMethod(barClass, "test")
val testInstructions = instructionsFromMethod(testMethod).filter(_.isInstanceOf[Invoke])
assertSameCode(testInstructions, List(
Invoke(INVOKEVIRTUAL, "foo/D$", "inline$bazImpl", "()V", false)))
}
}
}

object invocationReceiversTestCode {
Expand Down
6 changes: 6 additions & 0 deletions library/src/scala/annotation/inlineAccessible.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package scala.annotation

/** TODO
*/
@experimental
final class inlineAccessible extends StaticAnnotation
7 changes: 7 additions & 0 deletions tests/neg/inline-accessible.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package foo

import scala.annotation.inlineAccessible

@inlineAccessible type A // error
@inlineAccessible object B // error // TODO support?
@inlineAccessible class C // error
8 changes: 8 additions & 0 deletions tests/pos-macros/i15413/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import scala.quoted.*
import scala.annotation.inlineAccessible

class Macro:
inline def foo = ${ Macro.fooImpl }

object Macro:
@inlineAccessible private def fooImpl(using Quotes) = '{}
2 changes: 2 additions & 0 deletions tests/pos-macros/i15413/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def test =
new Macro().foo
6 changes: 6 additions & 0 deletions tests/pos-macros/i15413b/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import scala.quoted.*
import scala.annotation.inlineAccessible

inline def foo = ${ fooImpl }

@inlineAccessible private def fooImpl(using Quotes) = '{}
1 change: 1 addition & 0 deletions tests/pos-macros/i15413b/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
def test = foo
61 changes: 61 additions & 0 deletions tests/pos/inline-accessible.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// package foo

import scala.annotation.inlineAccessible

class Foo(@inlineAccessible param: Int):
@inlineAccessible
private val privateVal: Int = 2

@inlineAccessible
protected val protectedVal: Int = 2

@inlineAccessible
private[foo] def packagePrivateVal: Int = 2
inline def foo: Int = param + privateVal + protectedVal + packagePrivateVal

class Bar() extends Foo(3):
override protected val protectedVal: Int = 2

override private[foo] def packagePrivateVal: Int = 2

inline def bar: Int = protectedVal + packagePrivateVal // TODO reuse accessor in Foo

class Baz() extends Foo(4):
@inlineAccessible // TODO warn? : does not need an accessor and it overrides a field that has an accessor.
override protected val protectedVal: Int = 2

@inlineAccessible
override private[foo] def packagePrivateVal: Int = 2

inline def baz: Int = protectedVal + packagePrivateVal


class Qux() extends Foo(5):
inline def qux: Int = protectedVal + packagePrivateVal

def test =
@inlineAccessible // noop
val a = 5

Foo(3).foo
Bar().bar
Baz().baz
Qux().qux


package inlines {
// Case that needed to be converted with MakeInlineablePassing
class C[T](x: T) {
@inlineAccessible private[inlines] def next[U](y: U): (T, U) = (x, y)
}
class TestPassing {
inline def foo[A](x: A): (A, Int) = {
val c = new C[A](x)
c.next(1)
}
inline def bar[A](x: A): (A, String) = {
val c = new C[A](x)
c.next("")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ val experimentalDefinitionInLibrary = Set(
//// New feature: into
"scala.annotation.allowConversions",

//// Explicit inline accessors
"scala.annotation.inlineAccessible",

//// New APIs: Mirror
// Can be stabilized in 3.3.0 or later.
"scala.deriving.Mirror$.fromProductTyped", // This API is a bit convoluted. We may need some more feedback before we can stabilize it.
Expand Down

0 comments on commit eaf0804

Please sign in to comment.