Skip to content

Commit f09ba1b

Browse files
committed
Fix side-effect handling in reduceProjection
There's a difference to be made between projections of instance creations in a preceding val on the one hand and instance creations in a def or directly written creations on the other hand.
1 parent d786b4c commit f09ba1b

File tree

1 file changed

+33
-19
lines changed

1 file changed

+33
-19
lines changed

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

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -463,29 +463,35 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
463463
* - the class `C`
464464
* - the arguments `args`
465465
* - any bindings that wrap the instance creation
466+
* - whether the instance creation is precomputed or by-name
466467
*/
467468
private object NewInstance {
468-
def unapply(tree: Tree)(implicit ctx: Context): Option[(Symbol, List[Tree], List[Tree])] = {
469+
def unapply(tree: Tree)(implicit ctx: Context): Option[(Symbol, List[Tree], List[Tree], Boolean)] = {
469470
def unapplyLet(bindings: List[Tree], expr: Tree) =
470471
unapply(expr) map {
471-
case (cls, reduced, prefix) => (cls, reduced, bindings ::: prefix)
472+
case (cls, reduced, prefix, precomputed) => (cls, reduced, bindings ::: prefix, precomputed)
472473
}
473474
tree match {
474475
case Apply(fn, args) =>
475476
fn match {
476477
case Select(New(tpt), nme.CONSTRUCTOR) =>
477-
Some((tpt.tpe.classSymbol, args, Nil))
478+
Some((tpt.tpe.classSymbol, args, Nil, false))
478479
case TypeApply(Select(New(tpt), nme.CONSTRUCTOR), _) =>
479-
Some((tpt.tpe.classSymbol, args, Nil))
480+
Some((tpt.tpe.classSymbol, args, Nil, false))
480481
case _ =>
481482
val meth = fn.symbol
482483
if (meth.name == nme.apply &&
483484
meth.flags.is(Synthetic) &&
484-
meth.owner.linkedClass.is(Case)) Some(meth.owner.linkedClass, args, Nil)
485+
meth.owner.linkedClass.is(Case))
486+
Some(meth.owner.linkedClass, args, Nil, false)
485487
else None
486488
}
487489
case Ident(_) =>
488-
inlineBindings.get(tree.symbol).flatMap(unapply)
490+
for {
491+
binding <- inlineBindings.get(tree.symbol)
492+
(cls, reduced, prefix, precomputed) <- unapply(binding)
493+
}
494+
yield (cls, reduced, prefix, precomputed || binding.isInstanceOf[ValDef])
489495
case Inlined(_, bindings, expansion) =>
490496
unapplyLet(bindings, expansion)
491497
case Block(stats, expr) if isPureExpr(tree) =>
@@ -498,13 +504,13 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
498504

499505
/** If `tree` is equivalent to `new C(args).x` where class `C` does not have
500506
* initialization code and `x` is a parameter corresponding to one of the
501-
* arguments `args`, the corresponding argument, prefixed by the evaluation
502-
* of impure arguments, otherwise `tree` itself.
507+
* arguments `args`, the corresponding argument, otherwise `tree` itself.
508+
* Side effects of original arguments need to be preserved.
503509
*/
504510
def reduceProjection(tree: Tree)(implicit ctx: Context): Tree = {
505511
if (ctx.debug) inlining.println(i"try reduce projection $tree")
506512
tree match {
507-
case Select(NewInstance(cls, args, prefix), field) if cls.isNoInitsClass =>
513+
case Select(NewInstance(cls, args, prefix, precomputed), field) if cls.isNoInitsClass =>
508514
def matches(param: Symbol, selection: Symbol): Boolean =
509515
param == selection || {
510516
selection.name match {
@@ -516,17 +522,25 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(implicit ctx: Context) {
516522
}
517523
val idx = cls.asClass.paramAccessors.indexWhere(matches(_, tree.symbol))
518524
if (idx >= 0 && idx < args.length) {
519-
def collectImpure(from: Int, end: Int) =
520-
(from until end).filterNot(i => isPureExpr(args(i))).toList.map(args)
521-
val leading = collectImpure(0, idx)
522-
val trailing = collectImpure(idx + 1, args.length)
525+
def finish(arg: Tree) =
526+
new TreeTypeMap().transform(arg) // make sure local bindings in argument have fresh symbols
527+
.reporting(res => i"projecting $tree -> $res", inlining)
523528
val arg = args(idx)
524-
val argInPlace =
525-
if (trailing.isEmpty) arg
526-
else letBindUnless(TreeInfo.Pure, arg)(seq(trailing, _))
527-
val fullArg = seq(prefix, seq(leading, argInPlace))
528-
new TreeTypeMap().transform(fullArg) // make sure local bindings in argument have fresh symbols
529-
.reporting(res => i"projecting $tree -> $res", inlining)
529+
if (precomputed)
530+
if (isPureExpr(arg)) finish(arg)
531+
else tree // nothing we can do here, projection would duplicate side effect
532+
else {
533+
// newInstance is evaluated in place, need to reflect side effects of
534+
// arguments in the order they were written originally
535+
def collectImpure(from: Int, end: Int) =
536+
(from until end).filterNot(i => isPureExpr(args(i))).toList.map(args)
537+
val leading = collectImpure(0, idx)
538+
val trailing = collectImpure(idx + 1, args.length)
539+
val argInPlace =
540+
if (trailing.isEmpty) arg
541+
else letBindUnless(TreeInfo.Pure, arg)(seq(trailing, _))
542+
finish(seq(prefix, seq(leading, argInPlace)))
543+
}
530544
}
531545
else tree
532546
case Block(stats, expr) if stats.forall(isPureBinding) =>

0 commit comments

Comments
 (0)