Skip to content

Generate static inline accessors in the outermost scope #16982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 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,9 +58,12 @@ 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)
if site.isExtensibleClass then accName.expandedName(site) else accName
def accessorNameOf(accessed: Symbol, site: Symbol, isStatic: Boolean)(using Context): TermName =
val baseName =
if isStatic then ("static$" + accessed.fullName.toString.replace('.', '$')).toTermName
else accessed.name.asTermName
val accName = InlineAccessorName(baseName)
if site.isExtensibleClass && !isStatic then accName.expandedName(site) else accName

/** A definition needs an accessor if it is private, protected, or qualified private
* and it is not part of the tree that gets inlined. The latter test is implemented
Expand Down Expand Up @@ -99,18 +107,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) {
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 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 @@ -226,8 +248,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
27 changes: 13 additions & 14 deletions compiler/src/dotty/tools/dotc/transform/AccessProxies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ abstract class AccessProxies {
trait Insert {
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
/** The name of the accessor for definition `accessed` in given `site` */
def accessorNameOf(accessed: Symbol, 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,19 +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)(using Context): Tree = {
def useAccessor(reference: RefTree, accessorClass: Symbol, isStatic: Boolean = false)(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 accessorName = accessorNameOf(accessed, 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 All @@ -152,7 +151,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
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/transform/MixinOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
(meth.name == nme.readResolve || meth.name == nme.writeReplace) && meth.info.paramNamess.flatten.isEmpty

!meth.isConstructor &&
!meth.is(JavaStatic) &&
meth.is(Method, butNot = PrivateOrAccessorOrDeferred) &&
(ctx.settings.mixinForwarderChoices.isTruthy || meth.owner.is(Scala2x) || needsDisambiguation || hasNonInterfaceDefinition ||
generateJUnitForwarder || generateSerializationForwarder) &&
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(accessed: Symbol, site: Symbol, isStatic: Boolean)(using Context): TermName =
assert(!isStatic)
ProtectedAccessorName(accessed.name.asTermName)
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
123 changes: 123 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,129 @@ 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, "inline$static$foo$Baz") // FIXME "foo$$inline$static$Baz"?
assert(accessorNew.signature == "()Lfoo/Baz$;", accessorNew.signature)
assert((accessorNew.access & privateAccessors) == 0)
}
}

@Test
def i13215b(): Unit = {
val code =
"""package foo:
| final class 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, "inline$Baz")
assert(accessorOld.signature == "()Lfoo/Baz$;", accessorOld.signature)
assert((accessorOld.access & privateAccessors) == 0)

// For 3.4+
val accessorNew = getMethod(barClass, "inline$static$foo$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, "inline$static$Macro$$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, "inline$static$foo$D$$bazImpl") // FIXME "foo$D$$inline$static$bazImpl"?
assert(accessorNew.signature == "()V", accessorNew.signature)
assert((accessorNew.access & privateAccessors) == 0)
}
}

@Test
def i15413c(): Unit = {
val code =
"""package foo
|final 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, "inline$static$foo$D$$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 = ${ pack.Foo.impl }

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

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

package pack:
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
Loading