Skip to content

Commit 719729a

Browse files
oderskyodersky
committed
Fix #4322: Avoid generating multiple inline accessors for the same access
1 parent 6f6816d commit 719729a

File tree

1 file changed

+67
-54
lines changed

1 file changed

+67
-54
lines changed

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

Lines changed: 67 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ object Inliner {
4747
*/
4848
object addAccessors extends TreeMap {
4949
val accessors = new mutable.ListBuffer[MemberDef]
50+
private val getters = new mutable.HashMap[Symbol, Tree]
51+
private val setters = new mutable.HashMap[Symbol, Tree]
5052

5153
/** A definition needs an accessor if it is private, protected, or qualified private
5254
* and it is not part of the tree that gets inlined. The latter test is implemented
@@ -88,66 +90,70 @@ object Inliner {
8890
def addAccessor(tree: Tree, refPart: Tree, targs: List[Tree], argss: List[List[Tree]],
8991
accessedType: Type, rhs: (Tree, List[Type], List[List[Tree]]) => Tree)(implicit ctx: Context): Tree = {
9092
val qual = qualifier(refPart)
93+
9194
def refIsLocal = qual match {
9295
case qual: This => qual.symbol == refPart.symbol.owner
9396
case _ => false
9497
}
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-
}
11398

114-
val qualType = dealiasMap(qual.tpe.widen)
99+
def addAcc(accessorDef: MemberDef) = {
100+
accessors += accessorDef
101+
inlining.println(i"added inline accessor: $accessorDef")
102+
}
115103

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-
}
104+
if (refPart.symbol.isStatic || refIsLocal) {
105+
// Easy case: Reference to a static symbol or a symbol referenced via `this.`
106+
val accessorType = accessedType.ensureMethodic
107+
val accessor = accessorSymbol(tree, accessorType).asTerm
122108

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
109+
addAcc(
110+
polyDefDef(accessor, tps => argss =>
111+
rhs(refPart, tps, argss).withPos(tree.pos.focus)))
127112

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

134-
val accessorType = abstractQualType(addQualType(dealiasMap(accessedType)))
135-
val accessor = accessorSymbol(tree, accessorType).asTerm
134+
// The types that are local to the inlined method, and that therefore have
135+
// to be abstracted out in the accessor, which is external to the inlined method
136+
val localRefs = qualType.namedPartsWith(ref =>
137+
ref.isType && ref.symbol.isContainedIn(inlineMethod)).toList
136138

137-
val accessorDef = polyDefDef(accessor, tps => argss =>
139+
// Abstract accessed type over local refs
140+
def abstractQualType(mtpe: Type): Type =
141+
if (localRefs.isEmpty) mtpe
142+
else PolyType.fromParams(localRefs.map(_.symbol.asType), mtpe)
143+
.asInstanceOf[PolyType].flatten
144+
145+
val accessorType = abstractQualType(addQualType(dealiasMap(accessedType)))
146+
val accessor = accessorSymbol(tree, accessorType).asTerm
147+
148+
addAcc(
149+
polyDefDef(accessor, tps => argss =>
138150
rhs(argss.head.head.select(refPart.symbol), tps.drop(localRefs.length), argss.tail)
139-
.withPos(tree.pos.focus)
140-
)
151+
.withPos(tree.pos.focus)))
141152

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
153+
ref(accessor)
154+
.appliedToTypeTrees(localRefs.map(TypeTree(_)) ++ targs)
155+
.appliedToArgss((qual :: Nil) :: argss)
156+
}
151157
}
152158

153159
override def transform(tree: Tree)(implicit ctx: Context): Tree = super.transform {
@@ -158,12 +164,17 @@ object Inliner {
158164
if (methPart.symbol.isConstructor && needsAccessor(methPart.symbol)) {
159165
ctx.error("Cannot use private constructors in inline methods", tree.pos)
160166
tree // TODO: create a proper accessor for the private constructor
161-
} else {
162-
addAccessor(tree, methPart, targs, argss,
167+
}
168+
else {
169+
def addAcc =
170+
addAccessor(tree, methPart, targs, argss,
163171
accessedType = methPart.tpe.widen,
164-
rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss))
172+
rhs = (qual, tps, argss) => qual.appliedToTypes(tps).appliedToArgss(argss)))
173+
(if (targs.isEmpty && argss.isEmpty) getters.getOrElseUpdate(methPart.symbol, addAcc)
174+
else addAcc).withPos(tree.pos)
165175
}
166-
} else {
176+
}
177+
else {
167178
// TODO: Handle references to non-public types.
168179
// This is quite tricky, as such types can appear anywhere, including as parts
169180
// of types of other things. For the moment we do nothing and complain
@@ -177,9 +188,11 @@ object Inliner {
177188
tree
178189
}
179190
case Assign(lhs: RefTree, rhs) if needsAccessor(lhs.symbol) =>
180-
addAccessor(tree, lhs, Nil, (rhs :: Nil) :: Nil,
191+
setters.getOrElseUpdate(lhs.symbol,
192+
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+
.withPos(tree.pos)
183196
case _ => tree
184197
}
185198
}

0 commit comments

Comments
 (0)