Skip to content

Commit

Permalink
Generate static inline accessors module
Browse files Browse the repository at this point in the history
If a class needs inline accessors that would be added top-level
or if the accessor is to a static member, we place it in the top-level
class as a java static method.

If the accessor location in the new scheme is not the same as the
previous location, we also generate the old accessor for backward binary
compatibility but do not use it.

Fixes scala#13215
Fixes scala#15413
  • Loading branch information
nicolasstucki committed Feb 21, 2023
1 parent 72df9cd commit 7542e95
Show file tree
Hide file tree
Showing 34 changed files with 324 additions and 19 deletions.
31 changes: 26 additions & 5 deletions compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package dotty.tools
package dotc
package inlines

import scala.collection.mutable

import dotty.tools.dotc.ast.{Trees, tpd, untpd}
import Trees._
import core._
Expand All @@ -22,6 +24,9 @@ import transform.SymUtils.*
import config.Printers.inlining
import util.Property
import dotty.tools.dotc.transform.TreeMapWithStages._
import dotty.tools.dotc.ast.desugar.packageObjectName
import dotty.tools.dotc.core.StdNames.str
import dotty.tools.dotc.util.{Spans, SrcPos}

object PrepareInlineable {
import tpd._
Expand Down Expand Up @@ -53,8 +58,8 @@ object PrepareInlineable {
/** A tree map which inserts accessors for non-public term members accessed from inlined code.
*/
abstract class MakeInlineableMap(val inlineSym: Symbol) extends TreeMap with Insert {
def accessorNameOf(name: TermName, site: Symbol)(using Context): TermName =
val accName = InlineAccessorName(name)
def accessorNameOf(name: TermName, site: Symbol, isStatic: Boolean)(using Context): TermName =
val accName = InlineAccessorName(if isStatic then s"static$$$name".toTermName else name)
if site.isExtensibleClass then accName.expandedName(site) else accName

/** A definition needs an accessor if it is private, protected, or qualified private
Expand Down Expand Up @@ -99,20 +104,32 @@ object PrepareInlineable {
* 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
* by the test that we can find a host for the accessor.
*
* @param inlineSym symbol of the inline method
* @param compat use inline accessor format of 3.0-3.3
*/
class MakeInlineableDirect(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
class MakeInlineableDirect(inlineSym: Symbol, compat: Boolean) extends MakeInlineableMap(inlineSym) {
def preTransform(tree: Tree)(using Context): Tree = tree match {
case tree: RefTree if needsAccessor(tree.symbol) =>
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
else if compat then
// Generate the accessor for backwards compatibility with 3.0-3.3
val nearestHost = AccessProxies.hostForAccessorOf(tree.symbol)
val host = if nearestHost.is(Package) then ctx.owner.topLevelClass else nearestHost
useAccessor(tree, host)
else
// Generate the accessor for 3.4+
val nearestHost = AccessProxies.hostForAccessorOf(tree.symbol)
if nearestHost.is(Package) || (tree.symbol.owner.isStaticOwner && !nearestHost.isStaticOwner) then
useAccessor(tree, ctx.owner.topLevelClass, isStatic = true)
else
useAccessor(tree, nearestHost)
case _ =>
tree
}

override def ifNoHost(reference: RefTree)(using Context): Tree = reference
}

Expand Down Expand Up @@ -228,8 +245,12 @@ object PrepareInlineable {
// so no accessors are needed for them.
tree
else
// Generate inline accessors for 3.0-3.3
new MakeInlineablePassing(inlineSym).transform(
new MakeInlineableDirect(inlineSym, compat = true).transform(tree))
// Generate and use inline accessors for 3.4+
new MakeInlineablePassing(inlineSym).transform(
new MakeInlineableDirect(inlineSym).transform(tree))
new MakeInlineableDirect(inlineSym, compat = false).transform(tree))
}
}

Expand Down
12 changes: 8 additions & 4 deletions compiler/src/dotty/tools/dotc/quoted/Interpreter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,17 @@ class Interpreter(pos: SrcPos, classLoader0: ClassLoader)(using Context):
private def interpretVarargs(args: List[Object]): Object =
args.toSeq

private def interpretedStaticMethodCall(moduleClass: Symbol, fn: Symbol, args: List[Object]): Object = {
val inst =
try loadModule(moduleClass)
private def interpretedStaticMethodCall(owner: Symbol, fn: Symbol, args: List[Object]): Object = {
val (inst, clazz) =
try
if owner.is(Module) then
val inst = loadModule(owner)
(inst, inst.getClass)
else
(null, loadClass(owner.binaryClassName))
catch
case MissingClassDefinedInCurrentRun(sym) =>
suspendOnMissing(sym, pos)
val clazz = inst.getClass
val name = fn.name.asTermName
val method = getMethod(clazz, name, paramsSig(fn))
stopIfRuntimeException(method.invoke(inst, args: _*), method)
Expand Down
20 changes: 11 additions & 9 deletions compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ abstract class AccessProxies {
import ast.tpd._

/** The name of the accessor for definition with given `name` in given `site` */
def accessorNameOf(name: TermName, site: Symbol)(using Context): TermName
def accessorNameOf(name: TermName, site: Symbol, isStatic: Boolean)(using Context): TermName
def needsAccessor(sym: Symbol)(using Context): Boolean

def ifNoHost(reference: RefTree)(using Context): Tree = {
Expand All @@ -76,20 +76,21 @@ abstract class AccessProxies {
}

/** A fresh accessor symbol */
private def newAccessorSymbol(owner: Symbol, name: TermName, info: Type, accessed: Symbol)(using Context): TermSymbol = {
private def newAccessorSymbol(owner: Symbol, name: TermName, info: Type, accessed: Symbol, isStatic: Boolean)(using Context): TermSymbol = {
val sym = newSymbol(owner, name, Synthetic | Method, info, coord = accessed.span).entered
if accessed.is(Private) then sym.setFlag(Final)
if isStatic then sym.setFlag(JavaStaticTerm)
else if sym.allOverriddenSymbols.exists(!_.is(Deferred)) then sym.setFlag(Override)
if accessed.hasAnnotation(defn.ExperimentalAnnot) then
sym.addAnnotation(defn.ExperimentalAnnot)
sym
}

/** An accessor symbol, create a fresh one unless one exists already */
protected def accessorSymbol(owner: Symbol, accessorName: TermName, accessorInfo: Type, accessed: Symbol)(using Context): Symbol = {
protected def accessorSymbol(owner: Symbol, accessorName: TermName, accessorInfo: Type, accessed: Symbol, isStatic: Boolean = false)(using Context): Symbol = {
def refersToAccessed(sym: Symbol) = accessedBy.get(sym).contains(accessed)
owner.info.decl(accessorName).suchThat(refersToAccessed).symbol.orElse {
val acc = newAccessorSymbol(owner, accessorName, accessorInfo, accessed)
val acc = newAccessorSymbol(owner, accessorName, accessorInfo, accessed, isStatic)
accessedBy(acc) = accessed
acc
}
Expand Down Expand Up @@ -127,16 +128,17 @@ 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 Class where the accessor will be generated
* @param isStatic If this accessor will become a java static method
*/
def useAccessor(reference: RefTree, accessorClass: Symbol)(using Context): Tree = {
def useAccessor(reference: RefTree, accessorClass: Symbol, isStatic: Boolean = false)(using Context): Tree = {
val accessed = reference.symbol.asTerm
if (accessorClass.exists) {
val accessorName = accessorNameOf(accessed.name, accessorClass)
val accessorName = accessorNameOf(accessed.name, accessorClass, isStatic)
val accessorInfo =
accessed.info.ensureMethodic.asSeenFrom(accessorClass.thisType, accessed.owner)
val accessor = accessorSymbol(accessorClass, accessorName, accessorInfo, accessed)
val accessor = accessorSymbol(accessorClass, accessorName, accessorInfo, accessed, isStatic)
rewire(reference, accessor)
}
else ifNoHost(reference)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ class ProtectedAccessors extends MiniPhase {

private class Accessors extends AccessProxies {
val insert: Insert = new Insert {
def accessorNameOf(name: TermName, site: Symbol)(using Context): TermName = ProtectedAccessorName(name)
def accessorNameOf(name: TermName, site: Symbol, isStatic: Boolean)(using Context): TermName =
assert(!isStatic)
ProtectedAccessorName(name)
def needsAccessor(sym: Symbol)(using Context) = ProtectedAccessors.needsAccessor(sym)

override def ifNoHost(reference: RefTree)(using Context): Tree = {
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import util.common._
import util.{Property, SimpleIdentityMap, SrcPos}
import Applications.{tupleComponentTypes, wrapDefs, defaultArgument}


import collection.mutable
import annotation.tailrec
import Implicits._
Expand Down
74 changes: 74 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,80 @@ class DottyBytecodeTests extends DottyBytecodeTest {
assertSameCode(instructions, expected)
}
}

@Test
def i13215(): Unit = {
val code =
"""package foo:
| trait Bar:
| inline def baz = Baz
| private[foo] object Baz
""".stripMargin
checkBCode(code) { dir =>
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
val barClass = loadClassNode(dir.subdirectoryNamed("foo").lookupName("Bar.class", directory = false).input, skipDebugInfo = false)

// For 3.0-3.3 compat
val accessorOld = getMethod(barClass, "foo$Bar$$inline$Baz")
assert(accessorOld.signature == "()Lfoo/Baz$;", accessorOld.signature)
assert((accessorOld.access & privateAccessors) == 0)

// For 3.4+
val accessorNew = getMethod(barClass, "foo$Bar$$inline$static$Baz") // FIXME "foo$$inline$static$Baz"?
assert(accessorNew.signature == "()Lfoo/Baz$;", accessorNew.signature)
assert((accessorNew.access & privateAccessors) == 0)
}
}

@Test
def i15413(): Unit = {
val code =
"""import scala.quoted.*
|class Macro:
| inline def foo = Macro.fooImpl
|object Macro:
| private def fooImpl = {}
""".stripMargin
checkBCode(code) { dir =>
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
val macroClass = loadClassNode(dir.lookupName("Macro.class", directory = false).input, skipDebugInfo = false)

// For 3.0-3.3 compat
val accessorOld = getMethod(macroClass, "Macro$$inline$fooImpl")
assert(accessorOld.signature == "()V")
assert((accessorOld.access & privateAccessors) == 0)

// For 3.4+
val accessorNew = getMethod(macroClass, "Macro$$inline$static$fooImpl")
assert(accessorNew.signature == "()V")
assert((accessorNew.access & privateAccessors) == 0)
}
}

@Test
def i15413b(): Unit = {
val code =
"""package foo
|class C:
| inline def baz = D.bazImpl
|object D:
| private[foo] def bazImpl = {}
""".stripMargin
checkBCode(code) { dir =>
val privateAccessors = Opcodes.ACC_PRIVATE | Opcodes.ACC_PROTECTED
val barClass = loadClassNode(dir.subdirectoryNamed("foo").lookupName("C.class", directory = false).input, skipDebugInfo = false)

// For 3.0-3.3 compat
val accessorOld = getMethod(barClass, "inline$bazImpl$i1")
assert(accessorOld.desc == "(Lfoo/D$;)V", accessorOld.desc)
assert((accessorOld.access & privateAccessors) == 0)

// For 3.4+
val accessorNew = getMethod(barClass, "foo$C$$inline$static$bazImpl") // FIXME "foo$D$$inline$static$bazImpl"?
assert(accessorNew.signature == "()V", accessorNew.signature)
assert((accessorNew.access & privateAccessors) == 0)
}
}
}

object invocationReceiversTestCode {
Expand Down
7 changes: 7 additions & 0 deletions tests/pos-macros/i15413/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import scala.quoted.*

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

object Macro:
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
5 changes: 5 additions & 0 deletions tests/pos-macros/i15413b/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import scala.quoted.*

inline def foo = ${ fooImpl }

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
7 changes: 7 additions & 0 deletions tests/pos-macros/i15413c/Macro.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import scala.quoted.*

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

object Macro:
private def fooImpl(using Quotes) = '{}
2 changes: 2 additions & 0 deletions tests/pos-macros/i15413c/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def test =
new Macro().foo
5 changes: 5 additions & 0 deletions tests/pos-macros/i15413d/Macro.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import scala.quoted.*

inline def foo = ${ fooImpl }

private def fooImpl(using Quotes) = '{}
1 change: 1 addition & 0 deletions tests/pos-macros/i15413d/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
def test = foo
18 changes: 18 additions & 0 deletions tests/pos-macros/i15413e/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package myMacro

import scala.quoted.*

class Macro:
inline def foo = ${ Foo.impl }
inline def bar = ${ Bar.impl }
inline def baz = ${ foo.Foo.impl }

object Foo:
private[myMacro] def impl(using Quotes) = '{}

object Bar:
private[myMacro] def impl(using Quotes) = '{1}

package foo:
object Foo:
private[myMacro] def impl(using Quotes) = '{"abc"}
6 changes: 6 additions & 0 deletions tests/pos-macros/i15413e/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import myMacro.Macro

def test(m: Macro) =
m.foo
m.bar
m.baz
6 changes: 6 additions & 0 deletions tests/run-macros/i13215-compat-3.1/A_1_c3.1.0.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package foo

trait Bar:
inline def baz = Baz

private[foo] object Baz
6 changes: 6 additions & 0 deletions tests/run-macros/i13215-compat-3.1/A_3.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package foo

trait Bar:
inline def baz = Baz

private[foo] object Baz
25 changes: 25 additions & 0 deletions tests/run-macros/i13215-compat-3.1/B_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// We first compile A using 3.1 to generate the old accessors
// Then we compile this file to link against the old accessors
// Finally we recompile A using the current compiler to generate a version
// of A that contains the new accessors (and the old for backwards compat)

@main def Test =
val bar: foo.Bar = new foo.Bar{}
bar.baz // test that old accessor links in 3.4+

// Check that both accessors exist in the bytecode
val inlineAccessors =
java.lang.Class.forName("foo.Bar").getMethods()
.filter(_.getName().contains("inline"))
.filter(x => (x.getModifiers() & java.lang.reflect.Modifier.STATIC) == 0)
.map(_.getName())
.toList
val staticInlineAccessors =
java.lang.Class.forName("foo.Bar").getMethods()
.filter(_.getName().contains("inline"))
.filter(x => (x.getModifiers() & java.lang.reflect.Modifier.STATIC) != 0)
.map(_.getName())
.toList

println("3.0-3.3 inline accessor: " + inlineAccessors)
println("3.4+ inline accessor: " + staticInlineAccessors)
2 changes: 2 additions & 0 deletions tests/run-macros/i15413-compat-3.1.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
3.0-3.3 inline accessor: List(Macro$$inline$fooImpl)
3.4+ inline accessor: List(Macro$$inline$static$fooImpl)
7 changes: 7 additions & 0 deletions tests/run-macros/i15413-compat-3.1/Macro_1_c3.1.0.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import scala.quoted.*

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

object Macro:
private def fooImpl/*(using Quotes)*/ = {}
7 changes: 7 additions & 0 deletions tests/run-macros/i15413-compat-3.1/Macro_3.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import scala.quoted.*

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

object Macro:
private def fooImpl/*(using Quotes)*/ = {}
Loading

0 comments on commit 7542e95

Please sign in to comment.