Skip to content

Commit 2a5abb3

Browse files
oderskyodersky
committed
Fix #4322: Avoid generating more than one definition for an inline accessor
1 parent def8b69 commit 2a5abb3

File tree

2 files changed

+78
-55
lines changed

2 files changed

+78
-55
lines changed

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

Lines changed: 68 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ object Inliner {
4949
object addAccessors extends TreeMap {
5050
val accessors = new mutable.ListBuffer[MemberDef]
5151

52+
type AccessorMap = mutable.HashMap[Symbol, DefDef]
53+
private val getters = new AccessorMap
54+
private val setters = new AccessorMap
55+
5256
/** A definition needs an accessor if it is private, protected, or qualified private
5357
* and it is not part of the tree that gets inlined. The latter test is implemented
5458
* by excluding all symbols properly contained in the inlined method.
@@ -85,71 +89,77 @@ object Inliner {
8589
* @param rhs A function that builds the right-hand side of the accessor,
8690
* given a reference to the accessed symbol and any type and
8791
* value arguments the need to be integrated.
92+
* @param seen An map of already generated accessor methods of this kind (getter or setter)
8893
* @return The call to the accessor method that replaces the original access.
8994
*/
9095
def addAccessor(tree: Tree, refPart: Tree, targs: List[Tree], argss: List[List[Tree]],
91-
accessedType: Type, rhs: (Tree, List[Type], List[List[Tree]]) => Tree)(implicit ctx: Context): Tree = {
96+
accessedType: Type, rhs: (Tree, List[Type], List[List[Tree]]) => Tree,
97+
seen: AccessorMap)(implicit ctx: Context): Tree = {
9298
val qual = qualifier(refPart)
99+
93100
def refIsLocal = qual match {
94101
case qual: This => qual.symbol == refPart.symbol.owner
95102
case _ => false
96103
}
97-
val (accessorDef, accessorRef) =
98-
if (refPart.symbol.isStatic || refIsLocal) {
99-
// Easy case: Reference to a static symbol or a symbol referenced via `this.`
100-
val accessorType = accessedType.ensureMethodic
101-
val accessor = accessorSymbol(tree, accessorType).asTerm
102-
val accessorDef = polyDefDef(accessor, tps => argss =>
103-
rhs(refPart, tps, argss).withPos(tree.pos.focus))
104-
val accessorRef = ref(accessor).appliedToTypeTrees(targs).appliedToArgss(argss).withPos(tree.pos)
105-
(accessorDef, accessorRef)
106-
} else {
107-
// Hard case: Reference needs to go via a dynamic prefix
108-
inlining.println(i"adding inline accessor for $tree -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))")
109-
110-
// Need to dealias in order to catch all possible references to abstracted over types in
111-
// substitutions
112-
val dealiasMap = new TypeMap {
113-
def apply(t: Type) = mapOver(t.dealias)
114-
}
115104

116-
val qualType = dealiasMap(qual.tpe.widen)
105+
def accessorDef(accessorType: Type, accessorDefFn: TermSymbol => DefDef): DefDef =
106+
seen.getOrElseUpdate(refPart.symbol, {
107+
val acc = accessorSymbol(tree, accessorType).asTerm
108+
val accessorDef = accessorDefFn(acc)
109+
accessors += accessorDef
110+
inlining.println(i"added inline accessor: $accessorDef")
111+
accessorDef
112+
})
113+
114+
if (refPart.symbol.isStatic || refIsLocal) {
115+
// Easy case: Reference to a static symbol or a symbol referenced via `this.`
116+
val accDef = accessorDef(
117+
accessedType.ensureMethodic,
118+
polyDefDef(_, tps => argss => rhs(refPart, tps, argss).withPos(tree.pos.focus)))
117119

118-
// Add qualifier type as leading method argument to argument `tp`
119-
def addQualType(tp: Type): Type = tp match {
120-
case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, addQualType(tp.resultType))
121-
case tp: ExprType => addQualType(tp.resultType)
122-
case tp => MethodType(qualType :: Nil, tp)
123-
}
120+
ref(accDef.symbol).appliedToTypeTrees(targs).appliedToArgss(argss).withPos(tree.pos)
121+
}
122+
else {
123+
// Hard case: Reference needs to go via a dynamic prefix
124+
inlining.println(i"adding inline accessor for $tree -> (${qual.tpe}, $refPart: ${refPart.getClass}, [$targs%, %], ($argss%, %))")
125+
126+
// Need to dealias in order to catch all possible references to abstracted over types in
127+
// substitutions
128+
val dealiasMap = new TypeMap {
129+
def apply(t: Type) = mapOver(t.dealias)
130+
}
124131

125-
// The types that are local to the inlined method, and that therefore have
126-
// to be abstracted out in the accessor, which is external to the inlined method
127-
val localRefs = qualType.namedPartsWith(ref =>
128-
ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList
132+
val qualType = dealiasMap(qual.tpe.widen)
129133

130-
// Abstract accessed type over local refs
131-
def abstractQualType(mtpe: Type): Type =
132-
if (localRefs.isEmpty) mtpe
133-
else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe)
134-
.asInstanceOf[PolyType].flatten
134+
// Add qualifier type as leading method argument to argument `tp`
135+
def addQualType(tp: Type): Type = tp match {
136+
case tp: PolyType => tp.derivedLambdaType(tp.paramNames, tp.paramInfos, addQualType(tp.resultType))
137+
case tp: ExprType => addQualType(tp.resultType)
138+
case tp => MethodType(qualType :: Nil, tp)
139+
}
140+
141+
// The types that are local to the inlined method, and that therefore have
142+
// to be abstracted out in the accessor, which is external to the inlined method
143+
val localRefs = qualType.namedPartsWith(ref =>
144+
ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList
135145

136-
val accessorType = abstractQualType(addQualType(dealiasMap(accessedType)))
137-
val accessor = accessorSymbol(tree, accessorType).asTerm
146+
// Abstract accessed type over local refs
147+
def abstractQualType(mtpe: Type): Type =
148+
if (localRefs.isEmpty) mtpe
149+
else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe)
150+
.asInstanceOf[PolyType].flatten
138151

139-
val accessorDef = polyDefDef(accessor, tps => argss =>
152+
val accDef = accessorDef(
153+
abstractQualType(addQualType(dealiasMap(accessedType))),
154+
polyDefDef(_, tps => argss =>
140155
rhs(argss.head.head.select(refPart.symbol), tps.drop(localRefs.length), argss.tail)
141-
.withPos(tree.pos.focus)
142-
)
143-
144-
val accessorRef = ref(accessor)
145-
.appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs)
146-
.appliedToArgss((qual :: Nil) :: argss)
147-
.withPos(tree.pos)
148-
(accessorDef, accessorRef)
149-
}
150-
accessors += accessorDef
151-
inlining.println(i"added inline accessor: $accessorDef")
152-
accessorRef
156+
.withPos(tree.pos.focus)))
157+
158+
ref(accDef.symbol)
159+
.appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs)
160+
.appliedToArgss((qual :: Nil) :: argss)
161+
.withPos(tree.pos)
162+
}
153163
}
154164

155165
override def transform(tree: Tree)(implicit ctx: Context): Tree = super.transform {
@@ -160,12 +170,14 @@ object Inliner {
160170
if (methPart.symbol.isConstructor && needsAccessor(methPart.symbol)) {
161171
ctx.error("Cannot use private constructors in inline methods", tree.pos)
162172
tree // TODO: create a proper accessor for the private constructor
163-
} else {
173+
}
174+
else
164175
addAccessor(tree, methPart, targs, argss,
165176
accessedType = methPart.tpe.widen,
166-
rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss))
167-
}
168-
} else {
177+
rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss),
178+
seen = getters)
179+
}
180+
else {
169181
// TODO: Handle references to non-public types.
170182
// This is quite tricky, as such types can appear anywhere, including as parts
171183
// of types of other things. For the moment we do nothing and complain
@@ -181,7 +193,8 @@ object Inliner {
181193
case Assign(lhs: RefTree, rhs) if needsAccessor(lhs.symbol) =>
182194
addAccessor(tree, lhs, Nil, (rhs :: Nil) :: Nil,
183195
accessedType = MethodType(rhs.tpe.widen :: Nil, defn.UnitType),
184-
rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head))
196+
rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head),
197+
seen = setters)
185198
case _ => tree
186199
}
187200
}

‎tests/pos/i4322.scala‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
object Foo {
2+
private[this] var x: Int = 1
3+
4+
inline def foo: Int = x + x + x
5+
6+
inline def bar = {
7+
x += 1
8+
x += 1
9+
}
10+
}

0 commit comments

Comments
 (0)