Skip to content

Commit bae5449

Browse files
oderskyodersky
committed
Fix #4322: Avoid generating more than one definition for an inline accessor
1 parent 6f6816d commit bae5449

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
@@ -48,6 +48,10 @@ object Inliner {
4848
object addAccessors extends TreeMap {
4949
val accessors = new mutable.ListBuffer[MemberDef]
5050

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

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

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

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

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

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

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

153163
override def transform(tree: Tree)(implicit ctx: Context): Tree = super.transform {
@@ -158,12 +168,14 @@ object Inliner {
158168
if (methPart.symbol.isConstructor && needsAccessor(methPart.symbol)) {
159169
ctx.error("Cannot use private constructors in inline methods", tree.pos)
160170
tree // TODO: create a proper accessor for the private constructor
161-
} else {
171+
}
172+
else
162173
addAccessor(tree, methPart, targs, argss,
163174
accessedType = methPart.tpe.widen,
164-
rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss))
165-
}
166-
} else {
175+
rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss),
176+
seen = getters)
177+
}
178+
else {
167179
// TODO: Handle references to non-public types.
168180
// This is quite tricky, as such types can appear anywhere, including as parts
169181
// of types of other things. For the moment we do nothing and complain
@@ -179,7 +191,8 @@ object Inliner {
179191
case Assign(lhs: RefTree, rhs) if needsAccessor(lhs.symbol) =>
180192
addAccessor(tree, lhs, Nil, (rhs :: Nil) :: Nil,
181193
accessedType = MethodType(rhs.tpe.widen :: Nil, defn.UnitType),
182-
rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head))
194+
rhs = (lhs, tps, argss) => lhs.becomes(argss.head.head),
195+
seen = setters)
183196
case _ => tree
184197
}
185198
}

‎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)