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.

- Everything else happens in `Typers`: After typechecking a ValDef
  resulting from desugaring of a right-associative operator its Symbol
  and RHS are stored in a Map (without knowing at that point if they are
  by-name or by-value).

- After typechecking a method application with an Ident for one of these
  Symbols, check if the parameter is by-name in which case it is
  replaced with the RHS and the Symbol added to a Set.

- After typechecking a Block, check for a leading ValDef with the
  Symbol that was inlined and remove the ValDef.

Fixes scala/bug#1980

Author: szeiger

spec/06-expressions.md

@@ -698,9 +698,9 @@ This expression is then interpreted as $e.\mathit{op}(e_1,\ldots,e_n)$.
 
 A left-associative binary
 operation $e_1;\mathit{op};e_2$ is interpreted as $e_1.\mathit{op}(e_2)$. If $\mathit{op}$ is
-right-associative, the same operation is interpreted as
-`{ val $x$=$e_1$; $e_2$.$\mathit{op}$($x\,$) }`, where $x$ is a fresh
-name.
+right-associative and its parameter is passed by name, the same operation is interpreted as
+$e_2.\mathit{op}(e_1)$. If $\mathit{op}$ is right-associative and its parameter is passed by value,
+it is interpreted as `{ val $x$=$e_1$; $e_2$.$\mathit{op}$($x\,$) }`, where $x$ is a fresh name.
 
 ### Assignment Operators
 

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

@@ -835,7 +835,7 @@ self =>
         if (treeInfo.isLeftAssoc(op)) {
           Apply(mkSelection(left), arguments)
         } else {
-          val x = freshTermName()
+          val x = freshTermName(nme.RIGHT_ASSOC_OP_PREFIX)
           Block(
             List(ValDef(Modifiers(symtab.Flags.SYNTHETIC | symtab.Flags.ARTIFACT), x, TypeTree(), stripParens(left))),
             Apply(mkSelection(right), List(Ident(x))))

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

@@ -44,6 +44,11 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
 
   final val shortenImports = false
 
+  // All typechecked RHS of ValDefs for right-associative operator desugaring
+  private val rightAssocValDefs = new mutable.AnyRefMap[Symbol, Tree]
+  // Symbols of ValDefs for right-associative operator desugaring which are passed by name and have been inlined
+  private val inlinedRightAssocValDefs = new mutable.HashSet[Symbol]
+
   // allows override of the behavior of the resetTyper method w.r.t comments
   def resetDocComments() = {
     clearDocComments()
@@ -54,6 +59,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
     resetContexts()
     resetImplicits()
     resetDocComments()
+    rightAssocValDefs.clear()
+    inlinedRightAssocValDefs.clear()
   }
 
   sealed abstract class SilentResult[+T] {
@@ -2063,7 +2070,10 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           } else tpt1.tpe
           transformedOrTyped(vdef.rhs, EXPRmode | BYVALmode, tpt2)
         }
-      treeCopy.ValDef(vdef, typedMods, sym.name, tpt1, checkDead(rhs1)) setType NoType
+      val vdef1 = treeCopy.ValDef(vdef, typedMods, sym.name, tpt1, checkDead(rhs1)) setType NoType
+      if (sym.isSynthetic && sym.name.startsWith(nme.RIGHT_ASSOC_OP_PREFIX))
+        rightAssocValDefs += ((sym, vdef1.rhs))
+      vdef1
     }
 
     /** Enter all aliases of local parameter accessors.
@@ -2474,7 +2484,13 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           if (result0.nonEmpty) checkPure(result0, supple = true)
         }
 
-        treeCopy.Block(block, statsTyped, expr1)
+        // Remove ValDef for right-associative by-value operator desugaring which has been inlined into expr1
+        val statsTyped2 = statsTyped match {
+          case (vd: ValDef) :: Nil if inlinedRightAssocValDefs remove vd.symbol => Nil
+          case _ => statsTyped
+        }
+
+        treeCopy.Block(block, statsTyped2, expr1)
           .setType(if (treeInfo.isExprSafeToInline(block)) expr1.tpe else expr1.tpe.deconst)
       } finally {
         // enable escaping privates checking from the outside and recycle
@@ -3591,6 +3607,15 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
                   case _ => tp
                 }
 
+                // Inline RHS of ValDef for right-associative by-value operator desugaring
+                val args2 = (args1, mt.params) match {
+                  case ((ident: Ident) :: Nil, param :: Nil) if param.isByNameParam && rightAssocValDefs.contains(ident.symbol) =>
+                    inlinedRightAssocValDefs += ident.symbol
+                    val rhs = rightAssocValDefs.remove(ident.symbol).get
+                    rhs.changeOwner(ident.symbol -> context.owner) :: Nil
+                  case _ => args1
+                }
+
                 /*
                  * This is translating uses of List() into Nil.  This is less
                  *  than ideal from a consistency standpoint, but it shouldn't be
@@ -3602,7 +3627,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
                 if (args.isEmpty && canTranslateEmptyListToNil && fun.symbol.isInitialized && ListModule.hasCompleteInfo && (fun.symbol == List_apply))
                   atPos(tree.pos)(gen.mkNil setType restpe)
                 else
-                  constfold(treeCopy.Apply(tree, fun, args1) setType ifPatternSkipFormals(restpe))
+                  constfold(treeCopy.Apply(tree, fun, args2) setType ifPatternSkipFormals(restpe))
               }
               checkDead.updateExpr(fun) {
                 handleMonomorphicCall

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

@@ -333,6 +333,7 @@ trait StdNames {
     val FRESH_SUFFIX                   = "macro$" // uses a keyword to avoid collisions with mangled names
     val QUAL_PREFIX                    = "qual$"
     val NAMEDARG_PREFIX                = "x$"
+    val RIGHT_ASSOC_OP_PREFIX          = "rassoc$"
 
     // Compiler internal names
     val ANYname: NameType                  = "<anyname>"

test/files/run/t1980.check

@@ -0,0 +1,29 @@
+1. defining
+foo 1
+foo 2
+foo 3
+1. forcing
+bar 1
+bar 2
+bar 3
+2. defining
+2. forcing
+hi
+1
+hi
+1
+3. defining
+3. forcing
+hi
+1
+4. defining
+4. forcing
+1
+1
+1
+5. defining
+Int 1
+Int 3
+5. forcing
+String 4
+String 2

test/files/run/t1980.scala

@@ -0,0 +1,75 @@
+class LazyList[+A](expr: => LazyList.Evaluated[A]) {
+  def #:: [B >: A](elem: => B): LazyList[B] = new LazyList(Some((elem, this)))
+  def ##:: [B >: A](elem: B): LazyList[B] = new LazyList(Some((elem, this)))
+  def force: Unit = expr.foreach(_._2.force)
+}
+
+object LazyList {
+  type Evaluated[+A] = Option[(A, LazyList[A])]
+  object Empty extends LazyList[Nothing](None)
+  def empty[A]: LazyList[A] = Empty
+}
+
+object Test extends App {
+  def foo(i: Int) = { println("foo "+i); i }
+  def bar(i: Int) = { println("bar "+i); i }
+  println("1. defining")
+  val xs1 = foo(1) ##:: foo(2) ##:: foo(3) ##:: LazyList.empty
+  val xs2 = bar(1) #:: bar(2) #:: bar(3) #:: LazyList.empty
+  println("1. forcing")
+  xs1.force
+  xs2.force
+
+  {
+    println("2. defining")
+    class C { def f_:(x: => Int)(implicit y: Int): () => Int = (() => x) }
+    val c = new C
+    implicit val i = 1
+    def k = { println("hi"); 1 }
+    val x1 = c.f_:(k)
+    val x2 = k f_: c
+    println("2. forcing")
+    println(x1())
+    println(x2())
+  }
+
+  {
+    println("3. defining")
+    class C { def f_:[T](x: => T): () => T = (() => x) }
+    val c = new C
+    def k = { println("hi"); 1 }
+    val x1 = k f_:[Any] c
+    println("3. forcing")
+    println(x1())
+  }
+
+  // Ensure the owner chain is changed correctly when inlining
+  {
+    println("4. defining")
+    class C { def f_:(x: => Int): () => Int = (() => x) }
+    val c = new C
+    val x1 = c.f_:({ val xxx = 1; xxx })
+    val x2 = { val yyy = 1; yyy } f_: c
+    val x3 = { val yyy = 1; yyy } f_: c
+    println("4. forcing")
+    println(x1())
+    println(x2())
+    println(x3())
+  }
+
+  // Overloaded operator with by-name and by-value variants
+  {
+    println("5. defining")
+    class C {
+      val saved = new collection.mutable.ArrayBuffer[() => String]
+      def force: Unit = saved.foreach(_.apply())
+      def :: (x: Int): C = this
+      def :: (x: => String): C = { saved += (() => x); this }
+    }
+    def genI(i: Int): Int = { println("Int "+i); i }
+    def genS(s: String): String = { println("String "+s); s }
+    val c = genI(1) :: genS("2") :: genI(3) :: genS("4") :: (new C)
+    println("5. forcing")
+    c.force
+  }
+}