Skip to content

Commit fc046fb

Browse files
fhackettfhackett
committed
Fix scala#8306: Ensure the inliner can elide effectively pure case class applications in various situations
This is a collection of related changes, all to ensure that the inliner is capable of properly eliding case class constructions in various situations. Specific intended changes: - allow NewInstance.unapply to look past type ascriptions - change all purity checks to consider, for the inliner only, case class applications (when the apply method is synthetic and the case class has no constructor body) pure, or "elideable", despite the fact that the operation is almost always idempotent in the general case. - make reduceInlineMatch's reduceSubPatterns procedure set the defTree for the accessor projections it generates, so nested Unapply patterns can look in defTree to find and possible inline the appropriate subtree of the scrutinee expression - make the typedSelect override able to reduce projections without a corresponding inline match, so that if inlining produces SomeCaseClass(x=3).x the case class construction has a change to be elided (only in an inline context) This commit also includes a series of test cases which use compiletime.show to pretty-print the resulting inlined AST of a series of inline expansions. To avoid cases where a malformed AST pretty-prints but causes bad codegen, the computed run-time value is also printed. One last thing I tested manually was that the bytecode matched the pretty-printing. Mid-work on this commit I found that the pretty printer was hiding several cases where redundant code was left in but not showing up when pretty-printed. This is partially tested by the case where full inlining is not possible, which would also include a redundant binding for the scrutinee if that issue were still present.
1 parent 98103a2 commit fc046fb

File tree

3 files changed

+205
-10
lines changed

3 files changed

+205
-10
lines changed

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

Lines changed: 87 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
361361
computeParamBindings(tp.resultType, Nil, argss)
362362
case tp: MethodType =>
363363
if argss.isEmpty then
364-
ctx.error(i"mising arguments for inline method $inlinedMethod", call.sourcePos)
364+
ctx.error(i"missing arguments for inline method $inlinedMethod", call.sourcePos)
365365
false
366366
else
367367
tp.paramNames.lazyZip(tp.paramInfos).lazyZip(argss.head).foreach { (name, paramtp, arg) =>
@@ -414,6 +414,73 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
414414
tpe.cls.isContainedIn(inlinedMethod) ||
415415
tpe.cls.is(Package)
416416

417+
/** Very similar to TreeInfo.isPureExpr, but with the following inliner-only exceptions:
418+
* - synthetic case class apply methods, when the case class constructor is empty, are
419+
* elideable but not pure. Elsewhere, accessing the apply method might cause the initialization
420+
* of a containing object so they are merely idempotent.
421+
*/
422+
object isElideableExpr {
423+
def isStatElideable(tree: Tree)(implicit ctx: Context): Boolean = unsplice(tree) match {
424+
case EmptyTree
425+
| TypeDef(_, _)
426+
| Import(_, _)
427+
| DefDef(_, _, _, _, _) =>
428+
true
429+
case vdef @ ValDef(_, _, _) =>
430+
if (vdef.symbol.flags is Mutable) false else apply(vdef.rhs)
431+
case _ =>
432+
false
433+
}
434+
435+
def apply(tree: Tree): Boolean = unsplice(tree) match {
436+
case EmptyTree
437+
| This(_)
438+
| Super(_, _)
439+
| Literal(_) =>
440+
true
441+
case Ident(_) =>
442+
isPureRef(tree)
443+
case Select(qual, _) =>
444+
if (tree.symbol.is(Erased)) true
445+
else isPureRef(tree) && apply(qual)
446+
case New(_) | Closure(_, _, _) =>
447+
true
448+
case TypeApply(fn, _) =>
449+
if (fn.symbol.is(Erased) || fn.symbol == defn.InternalQuoted_typeQuote) true else apply(fn)
450+
case Apply(fn, args) =>
451+
def isKnownPureOp(sym: Symbol) =
452+
sym.owner.isPrimitiveValueClass
453+
|| sym.owner == defn.StringClass
454+
|| defn.pureMethods.contains(sym)
455+
val isCaseClassApply = {
456+
val cls = tree.tpe.classSymbol
457+
val meth = fn.symbol
458+
meth.name == nme.apply &&
459+
meth.flags.is(Synthetic) &&
460+
meth.owner.linkedClass.is(Case) &&
461+
cls.isNoInitsClass
462+
}
463+
if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol) // A constant expression with pure arguments is pure.
464+
|| (fn.symbol.isStableMember && !fn.symbol.is(Lazy))
465+
|| fn.symbol.isPrimaryConstructor && fn.symbol.owner.isNoInitsClass) // TODO: include in isStable?
466+
apply(fn) && args.forall(apply)
467+
else if (isCaseClassApply)
468+
args.forall(apply)
469+
else if (fn.symbol.is(Erased)) true
470+
else false
471+
case Typed(expr, _) =>
472+
apply(expr)
473+
case Block(stats, expr) =>
474+
apply(expr) && stats.forall(isStatElideable)
475+
case Inlined(_, bindings, expr) =>
476+
apply(expr) && bindings.forall(isStatElideable)
477+
case NamedArg(_, expr) =>
478+
apply(expr)
479+
case _ =>
480+
false
481+
}
482+
}
483+
417484
/** Populate `thisProxy` and `paramProxy` as follows:
418485
*
419486
* 1a. If given type refers to a static this, thisProxy binds it to corresponding global reference,
@@ -676,13 +743,16 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
676743
Some(meth.owner.linkedClass, args, Nil, false)
677744
else None
678745
}
746+
case Typed(inner, _) =>
747+
// drop the ascribed tpt. We only need it if we can't find a NewInstance
748+
unapply(inner)
679749
case Ident(_) =>
680750
val binding = tree.symbol.defTree
681751
for ((cls, reduced, prefix, precomputed) <- unapply(binding))
682752
yield (cls, reduced, prefix, precomputed || binding.isInstanceOf[ValDef])
683753
case Inlined(_, bindings, expansion) =>
684754
unapplyLet(bindings, expansion)
685-
case Block(stats, expr) if isPureExpr(tree) =>
755+
case Block(stats, expr) if isElideableExpr(tree) =>
686756
unapplyLet(stats, expr)
687757
case _ =>
688758
None
@@ -715,13 +785,13 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
715785
.reporting(i"projecting $tree -> $result", inlining)
716786
val arg = args(idx)
717787
if (precomputed)
718-
if (isPureExpr(arg)) finish(arg)
788+
if (isElideableExpr(arg)) finish(arg)
719789
else tree // nothing we can do here, projection would duplicate side effect
720790
else {
721791
// newInstance is evaluated in place, need to reflect side effects of
722792
// arguments in the order they were written originally
723793
def collectImpure(from: Int, end: Int) =
724-
(from until end).filterNot(i => isPureExpr(args(i))).toList.map(args)
794+
(from until end).filterNot(i => isElideableExpr(args(i))).toList.map(args)
725795
val leading = collectImpure(0, idx)
726796
val trailing = collectImpure(idx + 1, args.length)
727797
val argInPlace =
@@ -975,7 +1045,9 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
9751045
case (Nil, Nil) => true
9761046
case (pat :: pats1, selector :: selectors1) =>
9771047
val elem = newSym(InlineBinderName.fresh(), Synthetic, selector.tpe.widenTermRefExpr).asTerm
978-
caseBindingMap += ((NoSymbol, ValDef(elem, constToLiteral(selector)).withSpan(elem.span)))
1048+
val rhs = constToLiteral(selector)
1049+
elem.defTree = rhs
1050+
caseBindingMap += ((NoSymbol, ValDef(elem, rhs).withSpan(elem.span)))
9791051
reducePattern(caseBindingMap, elem.termRef, pat) &&
9801052
reduceSubPatterns(pats1, selectors1)
9811053
case _ => false
@@ -1077,9 +1149,14 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
10771149
override def typedSelect(tree: untpd.Select, pt: Type)(implicit ctx: Context): Tree = {
10781150
assert(tree.hasType, tree)
10791151
val qual1 = typed(tree.qualifier, selectionProto(tree.name, pt, this))
1080-
val res = constToLiteral(untpd.cpy.Select(tree)(qual1, tree.name).withType(tree.typeOpt))
1081-
ensureAccessible(res.tpe, tree.qualifier.isInstanceOf[untpd.Super], tree.sourcePos)
1082-
res
1152+
val resNoReduce = untpd.cpy.Select(tree)(qual1, tree.name).withType(tree.typeOpt)
1153+
val resMaybeReduced = constToLiteral(reducer.reduceProjection(resNoReduce))
1154+
if (resNoReduce ne resMaybeReduced)
1155+
typed(resMaybeReduced, pt) // redo typecheck if reduction changed something
1156+
else
1157+
val res = resMaybeReduced
1158+
ensureAccessible(res.tpe, tree.qualifier.isInstanceOf[untpd.Super], tree.sourcePos)
1159+
res
10831160
}
10841161

10851162
override def typedIf(tree: untpd.If, pt: Type)(implicit ctx: Context): Tree =
@@ -1192,8 +1269,8 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
11921269
val bindingOfSym = newMutableSymbolMap[MemberDef]
11931270

11941271
def isInlineable(binding: MemberDef) = binding match {
1195-
case ddef @ DefDef(_, Nil, Nil, _, _) => isPureExpr(ddef.rhs)
1196-
case vdef @ ValDef(_, _, _) => isPureExpr(vdef.rhs)
1272+
case ddef @ DefDef(_, Nil, Nil, _, _) => isElideableExpr(ddef.rhs)
1273+
case vdef @ ValDef(_, _, _) => isElideableExpr(vdef.rhs)
11971274
case _ => false
11981275
}
11991276
for (binding <- bindings if isInlineable(binding)) {

‎tests/run/i8306.check

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
compile-time: 3
2+
run-time: 3
3+
compile-time: 3
4+
run-time: 3
5+
compile-time: 3
6+
run-time: 3
7+
compile-time: 3
8+
run-time: 3
9+
compile-time: {
10+
val $elem9: A = Test.a
11+
val $elem10: Int = $elem9.i
12+
val i: Int = $elem10
13+
i:Int
14+
}
15+
run-time: 3
16+
compile-time: 3
17+
run-time: 3
18+
compile-time: 3
19+
run-time: 3
20+
compile-time: 3
21+
run-time: 3
22+
compile-time: 3
23+
run-time: 3
24+
compile-time: 3
25+
run-time: 3

‎tests/run/i8306.scala

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import scala.compiletime._
2+
3+
case class A(i: Int)
4+
case class B(a: A)
5+
case class C[T](t: T)
6+
7+
trait Test8 {
8+
inline def test8: Int =
9+
inline A(3) match {
10+
case A(i) => i
11+
}
12+
}
13+
14+
object Test extends Test8 {
15+
16+
inline def test1: Int =
17+
inline A(3) match {
18+
case A(i) => i
19+
}
20+
21+
inline def test2: Int =
22+
inline (A(3) : A) match {
23+
case A(i) => i
24+
}
25+
26+
inline def test3: Int =
27+
inline B(A(3)) match {
28+
case B(A(i)) => i
29+
}
30+
31+
inline def test4: Int =
32+
A(3).i
33+
34+
val a = A(3)
35+
inline def test5: Int =
36+
inline new B(a) match {
37+
case B(A(i)) => i
38+
}
39+
40+
inline def test6: Int =
41+
inline B(A(3)).a match {
42+
case A(i) => i
43+
}
44+
45+
inline def test7: Int =
46+
inline new A(3) match {
47+
case A(i) => i
48+
}
49+
50+
inline def test9: Int =
51+
B(A(3)).a.i
52+
53+
inline def test10: Int =
54+
inline C(3) match {
55+
case C(t) => t
56+
}
57+
58+
def main(argv: Array[String]): Unit = {
59+
println(code"compile-time: ${test1}")
60+
println(s"run-time: ${test1}")
61+
62+
println(code"compile-time: ${test2}")
63+
println(s"run-time: ${test2}")
64+
65+
println(code"compile-time: ${test3}")
66+
println(s"run-time: ${test3}")
67+
68+
println(code"compile-time: ${test4}")
69+
println(s"run-time: ${test4}")
70+
71+
// this is the only test that should not be possible to fully inline,
72+
// because it references a non-inline value
73+
println(code"compile-time: ${test5}")
74+
println(s"run-time: ${test5}")
75+
76+
println(code"compile-time: ${test6}")
77+
println(s"run-time: ${test6}")
78+
79+
println(code"compile-time: ${test7}")
80+
println(s"run-time: ${test7}")
81+
82+
println(code"compile-time: ${test8}")
83+
println(s"run-time: ${test8}")
84+
85+
println(code"compile-time: ${test9}")
86+
println(s"run-time: ${test9}")
87+
88+
// with type parameter
89+
println(code"compile-time: ${test10}")
90+
println(s"run-time: ${test10}")
91+
}
92+
}
93+

0 commit comments

Comments
 (0)