Skip to content

Commit 42913c9

Browse files
oderskyodersky
committed
Fix #4590: Reintroduce old inline accessor scheme as a fallback
Turns out things are not so simple. Some inline accessors cannot be handled in the same way as protected accessors because we cannot always place an accessor method next to the accessed symbol. We solve this by using an adapted version of the old inline accessor scheme as a fallback.
1 parent c3469da commit 42913c9

File tree

7 files changed

+214
-31
lines changed

7 files changed

+214
-31
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ object NameKinds {
291291
val LiftedTreeName = new UniqueNameKind("liftedTree")
292292
val SuperArgName = new UniqueNameKind("$superArg$")
293293
val DocArtifactName = new UniqueNameKind("$doc")
294+
val UniqueInlineName = new UniqueNameKind("$i")
294295

295296
/** A kind of unique extension methods; Unlike other unique names, these can be
296297
* unmangled.

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

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,20 +27,33 @@ abstract class AccessProxies {
2727
/** accessor -> accessed */
2828
private val accessedBy = newMutableSymbolMap[Symbol]
2929

30+
/** Given the name of an accessor, is the receiver of the call to accessed obtained
31+
* as a parameterer?
32+
*/
33+
protected def passReceiverAsArg(accessorName: Name)(implicit ctx: Context) = false
34+
3035
/** The accessor definitions that need to be added to class `cls`
3136
* As a side-effect, this method removes entries from the `accessedBy` map.
3237
* So a second call of the same method will yield the empty list.
3338
*/
3439
private def accessorDefs(cls: Symbol)(implicit ctx: Context): Iterator[DefDef] =
3540
for (accessor <- cls.info.decls.iterator; accessed <- accessedBy.remove(accessor)) yield
3641
polyDefDef(accessor.asTerm, tps => argss => {
37-
val accessRef = ref(TermRef(cls.thisType, accessed))
42+
def numTypeParams = accessed.info match {
43+
case info: PolyType => info.paramNames.length
44+
case _ => 0
45+
}
46+
val (accessRef, forwardedTypes, forwardedArgss) =
47+
if (passReceiverAsArg(accessor.name))
48+
(argss.head.head.select(accessed), tps.takeRight(numTypeParams), argss.tail)
49+
else
50+
(ref(TermRef(cls.thisType, accessed)), tps, argss)
3851
val rhs =
3952
if (accessor.name.isSetterName &&
40-
argss.nonEmpty && argss.head.nonEmpty) // defensive conditions
41-
accessRef.becomes(argss.head.head)
53+
forwardedArgss.nonEmpty && forwardedArgss.head.nonEmpty) // defensive conditions
54+
accessRef.becomes(forwardedArgss.head.head)
4255
else
43-
accessRef.appliedToTypes(tps).appliedToArgss(argss)
56+
accessRef.appliedToTypes(forwardedTypes).appliedToArgss(forwardedArgss)
4457
rhs.withPos(accessed.pos)
4558
})
4659

@@ -65,7 +78,7 @@ abstract class AccessProxies {
6578
}
6679

6780
/** An accessor symbol, create a fresh one unless one exists already */
68-
private def accessorSymbol(owner: Symbol, accessorName: TermName, accessorInfo: Type, accessed: Symbol)(implicit ctx: Context) = {
81+
protected def accessorSymbol(owner: Symbol, accessorName: TermName, accessorInfo: Type, accessed: Symbol)(implicit ctx: Context) = {
6982
def refersToAccessed(sym: Symbol) = accessedBy.get(sym).contains(accessed)
7083
owner.info.decl(accessorName).suchThat(refersToAccessed).symbol.orElse {
7184
val acc = newAccessorSymbol(owner, accessorName, accessorInfo, accessed.pos)
@@ -83,14 +96,24 @@ abstract class AccessProxies {
8396
}.withPos(reference.pos)
8497

8598
/** Given a reference to a getter accessor, the corresponding setter reference */
86-
def useSetter(getterRef: RefTree)(implicit ctx: Context): Tree = {
87-
val getter = getterRef.symbol
88-
val accessed = accessedBy(getter)
89-
val accessedName = accessed.name.asTermName
90-
val setterName = accessorNameKind(accessedName.setterName)
91-
val setterInfo = MethodType(getter.info.widenExpr :: Nil, defn.UnitType)
92-
val setter = accessorSymbol(getter.owner, setterName, setterInfo, accessed)
93-
rewire(getterRef, setter)
99+
def useSetter(getterRef: Tree)(implicit ctx: Context): Tree = getterRef match {
100+
case getterRef: RefTree =>
101+
val getter = getterRef.symbol.asTerm
102+
val accessed = accessedBy(getter)
103+
val setterName = getter.name.setterName
104+
def toSetterInfo(getterInfo: Type): Type = getterInfo match {
105+
case getterInfo: LambdaType =>
106+
getterInfo.derivedLambdaType(resType = toSetterInfo(getterInfo.resType))
107+
case _ =>
108+
MethodType(getterInfo :: Nil, defn.UnitType)
109+
}
110+
val setterInfo = toSetterInfo(getter.info.widenExpr)
111+
val setter = accessorSymbol(getter.owner, setterName, setterInfo, accessed)
112+
rewire(getterRef, setter)
113+
case Apply(fn, args) =>
114+
cpy.Apply(getterRef)(useSetter(fn), args)
115+
case TypeApply(fn, args) =>
116+
cpy.TypeApply(getterRef)(useSetter(fn), args)
94117
}
95118

96119
/** Create an accessor unless one exists already, and replace the original

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

Lines changed: 134 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import StdNames.nme
1515
import Contexts.Context
1616
import Names.{Name, TermName, EmptyTermName}
1717
import NameOps._
18-
import NameKinds.{ClassifiedNameKind, InlineAccessorName}
18+
import NameKinds.{ClassifiedNameKind, InlineAccessorName, UniqueInlineName}
1919
import ProtoTypes.selectionProto
2020
import SymDenotations.SymDenotation
2121
import Annotations._
@@ -33,10 +33,18 @@ object Inliner {
3333

3434
class InlineAccessors extends AccessProxies {
3535

36-
/** A tree map which inserts accessors for all non-public term members accessed
37-
* from inlined code. Accessors are collected in the `accessors` buffer.
38-
*/
39-
class MakeInlineable(inlineSym: Symbol) extends TreeMap with Insert {
36+
/** If an inline accessor name wraps a unique inline name, this is taken as indication
37+
* that the inline accessor takes its receiver as first parameter. Such accessors
38+
* are created by MakeInlineablePassing.
39+
*/
40+
override def passReceiverAsArg(name: Name)(implicit ctx: Context) = name match {
41+
case InlineAccessorName(UniqueInlineName(_, _)) => true
42+
case _ => false
43+
}
44+
45+
/** A tree map which inserts accessors for non-public term members accessed from inlined code.
46+
*/
47+
abstract class MakeInlineableMap(val inlineSym: Symbol) extends TreeMap with Insert {
4048
def accessorNameKind = InlineAccessorName
4149

4250
/** A definition needs an accessor if it is private, protected, or qualified private
@@ -48,18 +56,126 @@ object Inliner {
4856
(sym.is(AccessFlags) || sym.privateWithin.exists) &&
4957
!sym.isContainedIn(inlineSym)
5058

59+
def preTransform(tree: Tree)(implicit ctx: Context): Tree
60+
61+
def postTransform(tree: Tree)(implicit ctx: Context) = tree match {
62+
case Assign(lhs, rhs) if lhs.symbol.name.is(InlineAccessorName) =>
63+
cpy.Apply(tree)(useSetter(lhs), rhs :: Nil)
64+
case _ =>
65+
tree
66+
}
5167

52-
// TODO: Also handle references to non-public types.
53-
// This is quite tricky, as such types can appear anywhere, including as parts
54-
// of types of other things. For the moment we do nothing and complain
55-
// at the implicit expansion site if there's a reference to an inaccessible type.
5668
override def transform(tree: Tree)(implicit ctx: Context): Tree =
57-
super.transform(accessorIfNeeded(tree)) match {
58-
case tree1 @ Assign(lhs: RefTree, rhs) if lhs.symbol.name.is(InlineAccessorName) =>
59-
cpy.Apply(tree1)(useSetter(lhs), rhs :: Nil)
60-
case tree1 =>
61-
tree1
62-
}
69+
postTransform(super.transform(preTransform(tree)))
70+
}
71+
72+
/** Direct approach: place the accessor with the accessed symbol. This has the
73+
* advantage that we can re-use the receiver as is. But it is only
74+
* possible if the receiver is essentially this or an outer this, which is indicated
75+
* by the test that we can find a host for the accessor.
76+
*/
77+
class MakeInlineableDirect(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
78+
def preTransform(tree: Tree)(implicit ctx: Context): Tree = tree match {
79+
case tree: RefTree if needsAccessor(tree.symbol) =>
80+
if (tree.symbol.isConstructor) {
81+
ctx.error("Implementation restriction: cannot use private constructors in inline methods", tree.pos)
82+
tree // TODO: create a proper accessor for the private constructor
83+
}
84+
else if (AccessProxies.hostForAccessorOf(tree.symbol).exists) useAccessor(tree)
85+
else tree
86+
case _ =>
87+
tree
88+
}
89+
}
90+
91+
/** Fallback approach if the direct approach does not work: Place the accessor method
92+
* in the same class as the inlined method, and let it take the receiver as parameter.
93+
* This is tricky, since we have to find a suitable type for the parameter, which might
94+
* require additional type parameters for the inline accessor. An example is in the
95+
* `TestPassing` class in test `run/inline/inlines_1`:
96+
*
97+
* class C[T](x: T) {
98+
* private[inlines] def next[U](y: U): (T, U) = (x, y)
99+
* }
100+
* class TestPassing {
101+
* inline def foo[A](x: A): (A, Int) = {
102+
* val c = new C[A](x)
103+
* c.next(1)
104+
* }
105+
* inline def bar[A](x: A): (A, String) = {
106+
* val c = new C[A](x)
107+
* c.next("")
108+
* }
109+
*
110+
* `C` could be compiled separately, so we cannot place the inline accessor in it.
111+
* Instead, the inline accessor goes into `TestPassing` and takes the actual receiver
112+
* type as argument:
113+
*
114+
* def inline$next$i1[A, U](x$0: C[A])(y: U): (A, U) =
115+
* x$0.next[U](y)
116+
*
117+
* Since different calls might have different receiver types, we need to generate one
118+
* such accessor per call, so they need to have unique names.
119+
*/
120+
class MakeInlineablePassing(inlineSym: Symbol) extends MakeInlineableMap(inlineSym) {
121+
122+
def preTransform(tree: Tree)(implicit ctx: Context): Tree = tree match {
123+
case _: Apply | _: TypeApply | _: RefTree
124+
if needsAccessor(tree.symbol) && tree.isTerm && !tree.symbol.isConstructor =>
125+
val (refPart, targs, argss) = decomposeCall(tree)
126+
val qual = qualifier(refPart)
127+
inlining.println(i"adding receiver passing inline accessor for $tree -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))")
128+
129+
// Need to dealias in order to cagtch all possible references to abstracted over types in
130+
// substitutions
131+
val dealiasMap = new TypeMap {
132+
def apply(t: Type) = mapOver(t.dealias)
133+
}
134+
val qualType = dealiasMap(qual.tpe.widen)
135+
136+
// The types that are local to the inlined method, and that therefore have
137+
// to be abstracted out in the accessor, which is external to the inlined method
138+
val localRefs = qualType.namedPartsWith(ref =>
139+
ref.isType && ref.symbol.isContainedIn(inlineSym)).toList
140+
141+
// Add qualifier type as leading method argument to argument `tp`
142+
def addQualType(tp: Type): Type = tp match {
143+
case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, addQualType(tp.resultType))
144+
case tp: ExprType => addQualType(tp.resultType)
145+
case tp => MethodType(qualType :: Nil, tp)
146+
}
147+
148+
// Abstract accessed type over local refs
149+
def abstractQualType(mtpe: Type): Type =
150+
if (localRefs.isEmpty) mtpe
151+
else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe)
152+
.asInstanceOf[PolyType].flatten
153+
154+
val accessed = refPart.symbol.asTerm
155+
val accessedType = refPart.tpe.widen
156+
val accessor = accessorSymbol(
157+
owner = inlineSym.owner,
158+
accessorName = InlineAccessorName(UniqueInlineName.fresh(accessed.name)),
159+
accessorInfo = abstractQualType(addQualType(dealiasMap(accessedType))),
160+
accessed = accessed)
161+
162+
ref(accessor)
163+
.appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs)
164+
.appliedToArgss((qual :: Nil) :: argss)
165+
.withPos(tree.pos)
166+
167+
// TODO: Handle references to non-public types.
168+
// This is quite tricky, as such types can appear anywhere, including as parts
169+
// of types of other things. For the moment we do nothing and complain
170+
// at the implicit expansion site if there's a reference to an inaccessible type.
171+
// Draft code (incomplete):
172+
//
173+
// val accessor = accessorSymbol(tree, TypeAlias(tree.tpe)).asType
174+
// myAccessors += TypeDef(accessor).withPos(tree.pos.focus)
175+
// ref(accessor).withPos(tree.pos)
176+
//
177+
case _ => tree
178+
}
63179
}
64180

65181
/** Adds accessors for all non-public term members accessed
@@ -76,7 +192,9 @@ object Inliner {
76192
// Inline methods in local scopes can only be called in the scope they are defined,
77193
// so no accessors are needed for them.
78194
tree
79-
else new MakeInlineable(inlineSym).transform(tree)
195+
else
196+
new MakeInlineablePassing(inlineSym).transform(
197+
new MakeInlineableDirect(inlineSym).transform(tree))
80198
}
81199
}
82200

‎tests/pos/i4590.scala

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package test
2+
3+
class ArrayDeque {
4+
inline def isResizeNecessary(len: Int) = len > ArrayDeque.StableSize
5+
}
6+
7+
object ArrayDeque {
8+
private val StableSize = 256
9+
}
10+
11+
class List {
12+
inline def foo(x: List.Cons): Unit = {
13+
x.next = this
14+
}
15+
}
16+
object List {
17+
case class Cons(head: Int, private[List] var next: List)
18+
}
19+

‎tests/run/inline.check

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,5 @@ Outer.f Inner
77
Inner
88
Outer.f
99
Outer.f Inner
10+
(hi,1)
11+
(true,)

‎tests/run/inline/Test_2.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@ object Test {
1010

1111
val o = new Outer
1212
val i = new o.Inner
13+
val p = new TestPassing
1314
println(i.m)
1415
println(i.g)
1516
println(i.h)
1617
println(o.inner.m)
1718
println(o.inner.g)
1819
println(o.inner.h)
20+
println(p.foo("hi"))
21+
println(p.bar(true))
1922
}
20-
21-
}
23+
}

‎tests/run/inline/inlines_1.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,22 @@ object inlines {
3838
}
3939
val inner = new Inner
4040
}
41+
42+
class C[T](private[inlines] val x: T) {
43+
private[inlines] def next[U](y: U): (T, U) = (xx, y)
44+
private[inlines] var xx: T = _
45+
}
46+
47+
class TestPassing {
48+
inline def foo[A](x: A): (A, Int) = {
49+
val c = new C[A](x)
50+
c.xx = c.x
51+
c.next(1)
52+
}
53+
inline def bar[A](x: A): (A, String) = {
54+
val c = new C[A](x)
55+
c.xx = c.x
56+
c.next("")
57+
}
58+
}
4159
}

0 commit comments

Comments
 (0)