Skip to content

Commit 416ef51

Browse files
committed
Proper laziness for by-name args of right-associative operators
This fixes scala/bug#1980 by changing the desugaring of right-associative operator syntax in the spec such that by-name operands now get the same desugaring as left-associative operators (except for the reversed operands, of course). Only by-value operands are pulled out into intermediate vals to preserve their left-to-right evaluation order. The implementation is as follows: - `Parsers` still performs the `val` desugaring for all calls, except that the generated synthetic names use the new `RIGHT_ASSOC_OP_PREFIX` to identify them later. - When the operator method application is checked in `Typers`, the synthetic val’s `Symbol` is marked as `LAZY` if the parameter is by-name. - The `ValDef` of the marked `Symbol` is eliminated in `RefChecks` and its RHS inlined at the call site. This is rather hackish and there is no particular reason for doing it in `Typers` and `RefChecks` other than the desire to do it early and without the overhead of extra tree transforms.
1 parent 8e7c5c7 commit 416ef51

File tree

7 files changed

+55
-6
lines changed

7 files changed

+55
-6
lines changed

spec/06-expressions.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -698,9 +698,9 @@ This expression is then interpreted as $e.\mathit{op}(e_1,\ldots,e_n)$.
698698

699699
A left-associative binary
700700
operation $e_1;\mathit{op};e_2$ is interpreted as $e_1.\mathit{op}(e_2)$. If $\mathit{op}$ is
701-
right-associative, the same operation is interpreted as
702-
`{ val $x$=$e_1$; $e_2$.$\mathit{op}$($x\,$) }`, where $x$ is a fresh
703-
name.
701+
right-associative and its parameter is passed by name, the same operation is interpreted as
702+
$e_2.\mathit{op}(e_1)$. If $\mathit{op}$ is right-associative and its parameter is passed by value,
703+
it is interpreted as `{ val $x$=$e_1$; $e_2$.$\mathit{op}$($x\,$) }`, where $x$ is a fresh name.
704704

705705
### Assignment Operators
706706

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,7 @@ self =>
835835
if (treeInfo.isLeftAssoc(op)) {
836836
Apply(mkSelection(left), arguments)
837837
} else {
838-
val x = freshTermName()
838+
val x = freshTermName(nme.RIGHT_ASSOC_OP_PREFIX)
839839
Block(
840840
List(ValDef(Modifiers(symtab.Flags.SYNTHETIC | symtab.Flags.ARTIFACT), x, TypeTree(), stripParens(left))),
841841
Apply(mkSelection(right), List(Ident(x))))

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ package typechecker
99
import scala.language.postfixOps
1010

1111
import scala.collection.mutable
12-
import scala.collection.mutable.ListBuffer
12+
import scala.collection.mutable.{ListBuffer, HashMap}
1313
import scala.tools.nsc.settings.ScalaVersion
1414
import scala.tools.nsc.settings.NoScalaVersion
1515

@@ -36,6 +36,7 @@ import transform.Transform
3636
* <li>Local modules are replaced by variables and classes</li>
3737
* <li>Calls to case factory methods are replaced by new's.</li>
3838
* <li>Eliminate branches in a conditional if the condition is a constant</li>
39+
* <li>Synthetic vals from the desugaring of right-associative operators are inlined if they are passed by name<li>
3940
* </ul>
4041
*
4142
* @author Martin Odersky
@@ -116,6 +117,9 @@ abstract class RefChecks extends Transform {
116117

117118
var checkedCombinations = Set[List[Type]]()
118119

120+
// RHS of ValDefs for by-name RIGHT_ASSOC_OP_PREFIX calls that were eliminated. Subsequent calls are inlined.
121+
val eliminatedRightAssocValDefs = new HashMap[Symbol, Tree]
122+
119123
// only one overloaded alternative is allowed to define default arguments
120124
private def checkOverloadedRestrictions(clazz: Symbol, defaultClass: Symbol): Unit = {
121125
// Using the default getters (such as methodName$default$1) as a cheap way of
@@ -1703,7 +1707,7 @@ abstract class RefChecks extends Transform {
17031707
assert(sym != NoSymbol, "transformCaseApply: name = " + name.debugString + " tree = " + tree + " / " + tree.getClass) //debug
17041708
enterReference(tree.pos, sym)
17051709
}
1706-
tree
1710+
eliminatedRightAssocValDefs.getOrElse(tree.symbol, tree)
17071711

17081712
case x @ Select(_, _) =>
17091713
transformSelect(x)
@@ -1714,6 +1718,9 @@ abstract class RefChecks extends Transform {
17141718
// probably not, until we allow parameterised extractors
17151719
tree
17161720

1721+
case Block((vd: ValDef) :: Nil, expr) if vd.symbol.isLazy && vd.name.toString.startsWith(nme.RIGHT_ASSOC_OP_PREFIX) =>
1722+
eliminatedRightAssocValDefs += ((vd.symbol, vd.rhs))
1723+
transform(expr)
17171724

17181725
case _ => tree
17191726
}

src/compiler/scala/tools/nsc/typechecker/Typers.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3591,6 +3591,17 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
35913591
case _ => tp
35923592
}
35933593

3594+
// When a RIGHT_ASSOC_OP_PREFIX Ident is passed by name, set its Symbol's LAZY flag.
3595+
// The RHS then gets inlined and the ValDef removed in refchecks.
3596+
args1 match {
3597+
case (ident @ Ident(name)) :: Nil if name.toString.startsWith(nme.RIGHT_ASSOC_OP_PREFIX) =>
3598+
mt.params match {
3599+
case param :: Nil if param.isByNameParam => ident.symbol.setFlag(symtab.Flags.LAZY)
3600+
case _ =>
3601+
}
3602+
case _ =>
3603+
}
3604+
35943605
/*
35953606
* This is translating uses of List() into Nil. This is less
35963607
* than ideal from a consistency standpoint, but it shouldn't be

src/reflect/scala/reflect/internal/StdNames.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@ trait StdNames {
333333
val FRESH_SUFFIX = "macro$" // uses a keyword to avoid collisions with mangled names
334334
val QUAL_PREFIX = "qual$"
335335
val NAMEDARG_PREFIX = "x$"
336+
val RIGHT_ASSOC_OP_PREFIX = "rassoc$"
336337

337338
// Compiler internal names
338339
val ANYname: NameType = "<anyname>"

test/files/run/t1980.check

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
defining
2+
foo 1
3+
foo 2
4+
foo 3
5+
forcing
6+
bar 1
7+
bar 2
8+
bar 3

test/files/run/t1980.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
class LazyList[+A](expr: => LazyList.Evaluated[A]) {
2+
def #:: [B >: A](elem: => B): LazyList[B] = new LazyList(Some((elem, this)))
3+
def ##:: [B >: A](elem: B): LazyList[B] = new LazyList(Some((elem, this)))
4+
def force: Unit = expr.foreach(_._2.force)
5+
}
6+
7+
object LazyList {
8+
type Evaluated[+A] = Option[(A, LazyList[A])]
9+
object Empty extends LazyList[Nothing](None)
10+
def empty[A]: LazyList[A] = Empty
11+
}
12+
13+
object Test extends App {
14+
def foo(i: Int) = { println("foo "+i); i }
15+
def bar(i: Int) = { println("bar "+i); i }
16+
println("defining")
17+
val xs1 = foo(1) ##:: foo(2) ##:: foo(3) ##:: LazyList.empty
18+
val xs2 = bar(1) #:: bar(2) #:: bar(3) #:: LazyList.empty
19+
println("forcing")
20+
xs1.force
21+
xs2.force
22+
}

0 commit comments

Comments
 (0)