Fix #2939: Avoid duplicating symbols in default arguments

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

@@ -311,9 +311,9 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
   import tpd._
 
   /** The purity level of this statement.
-   *  @return   pure        if statement has no side effects
-   *            idempotent  if running the statement a second time has no side effects
-   *            impure      otherwise
+   *  @return   Pure        if statement has no side effects
+   *            Idempotent  if running the statement a second time has no side effects
+   *            Impure      otherwise
    */
   private def statPurity(tree: Tree)(implicit ctx: Context): PurityLevel = unsplice(tree) match {
     case EmptyTree
@@ -322,17 +322,18 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
        | DefDef(_, _, _, _, _) =>
       Pure
     case vdef @ ValDef(_, _, _) =>
-      if (vdef.symbol.flags is Mutable) Impure else exprPurity(vdef.rhs)
+      if (vdef.symbol.flags is Mutable) Impure else exprPurity(vdef.rhs) `min` Pure
     case _ =>
       Impure
       // TODO: It seem like this should be exprPurity(tree)
       // But if we do that the repl/vars test break. Need to figure out why that's the case.
   }
 
   /** The purity level of this expression.
-   *  @return   pure        if expression has no side effects
-   *            idempotent  if running the expression a second time has no side effects
-   *            impure      otherwise
+   *  @return   PurePath    if expression has no side effects and cannot contain local definitions
+   *            Pure        if expression has no side effects
+   *            Idempotent  if running the expression a second time has no side effects
+   *            Impure      otherwise
    *
    *  Note that purity and idempotency are different. References to modules and lazy
    *  vals are impure (side-effecting) both because side-effecting code may be executed and because the first reference
@@ -345,7 +346,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
        | Super(_, _)
        | Literal(_)
        | Closure(_, _, _) =>
-      Pure
+      PurePath
     case Ident(_) =>
       refPurity(tree)
     case Select(qual, _) =>
@@ -366,7 +367,7 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
       if (args.isEmpty && fn.symbol.is(Stable)) exprPurity(fn)
       else if (tree.tpe.isInstanceOf[ConstantType] && isKnownPureOp(tree.symbol))
         // A constant expression with pure arguments is pure.
-        minOf(exprPurity(fn), args.map(exprPurity))
+        minOf(exprPurity(fn), args.map(exprPurity)) `min` Pure
       else Impure
     case Typed(expr, _) =>
       exprPurity(expr)
@@ -382,25 +383,26 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
 
   private def minOf(l0: PurityLevel, ls: List[PurityLevel]) = (l0 /: ls)(_ min _)
 
-  def isPureExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) == Pure
+  def isPurePath(tree: Tree)(implicit ctx: Context) = exprPurity(tree) == PurePath
+  def isPureExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) >= Pure
   def isIdempotentExpr(tree: Tree)(implicit ctx: Context) = exprPurity(tree) >= Idempotent
 
   /** The purity level of this reference.
    *  @return
-   *    pure        if reference is (nonlazy and stable) or to a parameterized function
-   *    idempotent  if reference is lazy and stable
-   *    impure      otherwise
+   *    PurePath    if reference is (nonlazy and stable) or to a parameterized function
+   *    Idempotent  if reference is lazy and stable
+   *    Impure      otherwise
    *  @DarkDimius: need to make sure that lazy accessor methods have Lazy and Stable
    *               flags set.
    */
   private def refPurity(tree: Tree)(implicit ctx: Context): PurityLevel =
-    if (!tree.tpe.widen.isParameterless) Pure
+    if (!tree.tpe.widen.isParameterless) PurePath
     else if (!tree.symbol.isStable) Impure
     else if (tree.symbol.is(Lazy)) Idempotent // TODO add Module flag, sinxce Module vals or not Lazy from the start.
-    else Pure
+    else PurePath
 
   def isPureRef(tree: Tree)(implicit ctx: Context) =
-    refPurity(tree) == Pure
+    refPurity(tree) == PurePath
   def isIdempotentRef(tree: Tree)(implicit ctx: Context) =
     refPurity(tree) >= Idempotent
 
@@ -724,6 +726,7 @@ object TreeInfo {
     def min(that: PurityLevel) = new PurityLevel(x min that.x)
   }
 
+  val PurePath = new PurityLevel(3)
   val Pure = new PurityLevel(2)
   val Idempotent = new PurityLevel(1)
   val Impure = new PurityLevel(0)

compiler/src/dotty/tools/dotc/transform/PhantomArgLift.scala

@@ -5,7 +5,7 @@ import dotty.tools.dotc.core.Contexts._
 import dotty.tools.dotc.core.NameKinds._
 import dotty.tools.dotc.core.Types._
 import dotty.tools.dotc.transform.MegaPhase.MiniPhase
-import dotty.tools.dotc.typer.EtaExpansion
+import dotty.tools.dotc.typer.LiftImpure
 
 import scala.collection.mutable.ListBuffer
 
@@ -46,7 +46,7 @@ class PhantomArgLift extends MiniPhase {
       if (!hasImpurePhantomArgs(tree)) tree
       else {
         val buffer = ListBuffer.empty[Tree]
-        val app = EtaExpansion.liftApp(buffer, tree)
+        val app = LiftImpure.liftApp(buffer, tree)
         if (buffer.isEmpty) app
         else Block(buffer.result(), app)
       }

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

@@ -22,7 +22,6 @@ import Names._
 import StdNames._
 import NameKinds.DefaultGetterName
 import ProtoTypes._
-import EtaExpansion._
 import Inferencing._
 
 import collection.mutable
@@ -426,7 +425,8 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
           }
 
           def tryDefault(n: Int, args1: List[Arg]): Unit = {
-            if (!isJavaAnnotConstr(methRef.symbol)) liftFun()
+            if (!isJavaAnnotConstr(methRef.symbol))
+              liftFun()
             val getter = findDefaultGetter(n + numArgs(normalizedFun))
             if (getter.isEmpty) missingArg(n)
             else {
@@ -571,10 +571,13 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
 
     def normalizedFun = myNormalizedFun
 
+    private def lifter(implicit ctx: Context) =
+      if (methRef.symbol.hasDefaultParams) LiftComplex else LiftImpure
+
     override def liftFun(): Unit =
       if (liftedDefs == null) {
         liftedDefs = new mutable.ListBuffer[Tree]
-        myNormalizedFun = liftApp(liftedDefs, myNormalizedFun)
+        myNormalizedFun = lifter.liftApp(liftedDefs, myNormalizedFun)
       }
 
     /** The index of the first difference between lists of trees `xs` and `ys`
@@ -608,13 +611,13 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
 
             // lift arguments in the definition order
             val argDefBuf = mutable.ListBuffer.empty[Tree]
-            typedArgs = liftArgs(argDefBuf, methType, typedArgs)
+            typedArgs = lifter.liftArgs(argDefBuf, methType, typedArgs)
 
             // Lifted arguments ordered based on the original order of typedArgBuf and
             // with all non-explicit default parameters at the end in declaration order.
             val orderedArgDefs = {
               // List of original arguments that are lifted by liftArgs
-              val impureArgs = typedArgBuf.filterNot(isPureExpr)
+              val impureArgs = typedArgBuf.filterNot(lifter.noLift)
               // Assuming stable sorting all non-explicit default parameters will remain in the end with the same order
               val defaultParamIndex = args.size
               // Mapping of index of each `liftable` into original args ordering
@@ -746,7 +749,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
       val Apply(Select(lhs, name), rhss) = tree
       val lhs1 = typedExpr(lhs)
       val liftedDefs = new mutable.ListBuffer[Tree]
-      val lhs2 = untpd.TypedSplice(liftAssigned(liftedDefs, lhs1))
+      val lhs2 = untpd.TypedSplice(LiftComplex.liftAssigned(liftedDefs, lhs1))
       val assign = untpd.Assign(lhs2,
           untpd.Apply(untpd.Select(lhs2, name.asSimpleName.dropRight(1)), rhss))
       wrapDefs(liftedDefs, typed(assign))

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

@@ -13,23 +13,44 @@ import Decorators._
 import Names._
 import StdNames._
 import NameKinds.UniqueName
-import Trees._
 import Inferencing._
 import util.Positions._
 import collection.mutable
+import Trees._
 
-object EtaExpansion {
-
+/** A class that handles argument lifting. Argument lifting is needed in the following
+ *  scenarios:
+ *   - eta expansion
+ *   - applications with default arguments
+ *   - applications with out-of-order named arguments
+ *  Lifting generally lifts impure expressions only, except in the case of possible
+ *  default arguments, where we lift also complex pure expressions, since in that case
+ *  arguments can be duplicated as arguments to default argument methods.
+ */
+abstract class Lifter {
   import tpd._
 
+  /** Test indicating `expr` does not need lifting */
+  def noLift(expr: Tree)(implicit ctx: Context): Boolean
+
+  /** The corresponding lifter for paam-by-name arguments */
+  protected def exprLifter: Lifter = NoLift
+
+  /** The flags of a lifted definition */
+  protected def liftedFlags: FlagSet = EmptyFlags
+
+  /** The tree of a lifted definition */
+  protected def liftedDef(sym: TermSymbol, rhs: Tree)(implicit ctx: Context): MemberDef = ValDef(sym, rhs)
+
   private def lift(defs: mutable.ListBuffer[Tree], expr: Tree, prefix: TermName = EmptyTermName)(implicit ctx: Context): Tree =
-    if (isPureExpr(expr)) expr
+    if (noLift(expr)) expr
     else {
       val name = UniqueName.fresh(prefix)
-      val liftedType = fullyDefinedType(expr.tpe.widen, "lifted expression", expr.pos)
-      val sym = ctx.newSymbol(ctx.owner, name, EmptyFlags, liftedType, coord = positionCoord(expr.pos))
-      defs += ValDef(sym, expr).withPos(expr.pos.focus)
-      ref(sym.termRef).withPos(expr.pos)
+      var liftedType = fullyDefinedType(expr.tpe.widen, "lifted expression", expr.pos)
+      if (liftedFlags.is(Method)) liftedType = ExprType(liftedType)
+      val lifted = ctx.newSymbol(ctx.owner, name, liftedFlags, liftedType, coord = positionCoord(expr.pos))
+      defs += liftedDef(lifted, expr).withPos(expr.pos.focus)
+      ref(lifted.termRef).withPos(expr.pos)
     }
 
   /** Lift out common part of lhs tree taking part in an operator assignment such as
@@ -49,7 +70,7 @@ object EtaExpansion {
   }
 
   /** Lift a function argument, stripping any NamedArg wrapper */
-  def liftArg(defs: mutable.ListBuffer[Tree], arg: Tree, prefix: TermName = EmptyTermName)(implicit ctx: Context): Tree =
+  private def liftArg(defs: mutable.ListBuffer[Tree], arg: Tree, prefix: TermName = EmptyTermName)(implicit ctx: Context): Tree =
     arg match {
       case arg @ NamedArg(name, arg1) => cpy.NamedArg(arg)(name, lift(defs, arg1, prefix))
       case arg => lift(defs, arg, prefix)
@@ -61,12 +82,12 @@ object EtaExpansion {
   def liftArgs(defs: mutable.ListBuffer[Tree], methRef: Type, args: List[Tree])(implicit ctx: Context) =
     methRef.widen match {
       case mt: MethodType =>
-        (args, mt.paramNames, mt.paramInfos).zipped map { (arg, name, tp) =>
-          if (tp.isInstanceOf[ExprType]) arg
-          else liftArg(defs, arg, if (name.firstPart contains '$') EmptyTermName else name)
+        (args, mt.paramNames, mt.paramInfos).zipped.map { (arg, name, tp) =>
+          val lifter = if (tp.isInstanceOf[ExprType]) exprLifter else this
+          lifter.liftArg(defs, arg, if (name.firstPart contains '$') EmptyTermName else name)
         }
       case _ =>
-        args map (liftArg(defs, _))
+        args.map(liftArg(defs, _))
     }
 
   /** Lift out function prefix and all arguments from application
@@ -108,6 +129,35 @@ object EtaExpansion {
     case New(_) => tree
     case _ => if (isIdempotentExpr(tree)) tree else lift(defs, tree)
   }
+}
+
+/** No lifting at all */
+object NoLift extends Lifter {
+  def noLift(expr: tpd.Tree)(implicit ctx: Context) = true
+}
+
+/** Lift all impure arguments */
+class LiftImpure extends Lifter {
+  def noLift(expr: tpd.Tree)(implicit ctx: Context) = tpd.isPureExpr(expr)
+}
+object LiftImpure extends LiftImpure
+
+/** Lift all impure or complex arguments */
+class LiftComplex extends Lifter {
+  def noLift(expr: tpd.Tree)(implicit ctx: Context) = tpd.isPurePath(expr)
+  override def exprLifter = LiftToDefs
+}
+object LiftComplex extends LiftComplex
+
+/** Lift all impure or complex arguments to `def`s */
+object LiftToDefs extends LiftComplex {
+  override def liftedFlags: FlagSet = Method
+  override def liftedDef(sym: TermSymbol, rhs: tpd.Tree)(implicit ctx: Context) = tpd.DefDef(sym, rhs)
+}
+
+/** Lifter for eta expansion */
+object EtaExpansion extends LiftImpure {
+  import tpd._
 
   /** Eta-expanding a tree means converting a method reference to a function value.
    *  @param    tree       The tree to expand
@@ -152,41 +202,3 @@ object EtaExpansion {
     if (defs.nonEmpty) untpd.Block(defs.toList map (untpd.TypedSplice(_)), fn) else fn
   }
 }
-
-  /** <p> not needed
-   *    Expand partial function applications of type `type`.
-   *  </p><pre>
-   *  p.f(es_1)...(es_n)
-   *     ==>  {
-   *            <b>private synthetic val</b> eta$f   = p.f   // if p is not stable
-   *            ...
-   *            <b>private synthetic val</b> eta$e_i = e_i    // if e_i is not stable
-   *            ...
-   *            (ps_1 => ... => ps_m => eta$f([es_1])...([es_m])(ps_1)...(ps_m))
-   *          }</pre>
-   *  <p>
-   *    tree is already attributed
-   *  </p>
-  def etaExpandUntyped(tree: Tree)(implicit ctx: Context): untpd.Tree = { // kept as a reserve for now
-    def expand(tree: Tree): untpd.Tree = tree.tpe match {
-      case mt @ MethodType(paramNames, paramTypes) if !mt.isImplicit =>
-        val paramsArgs: List[(untpd.ValDef, untpd.Tree)] =
-          (paramNames, paramTypes).zipped.map { (name, tp) =>
-            val droppedStarTpe = defn.underlyingOfRepeated(tp)
-            val param = ValDef(
-              Modifiers(Param), name,
-              untpd.TypedSplice(TypeTree(droppedStarTpe)), untpd.EmptyTree)
-            var arg: untpd.Tree = Ident(name)
-            if (defn.isRepeatedParam(tp))
-              arg = Typed(arg, Ident(tpnme.WILDCARD_STAR))
-            (param, arg)
-          }
-        val (params, args) = paramsArgs.unzip
-        untpd.Function(params, Apply(untpd.TypedSplice(tree), args))
-    }
-
-    val defs = new mutable.ListBuffer[Tree]
-    val tree1 = liftApp(defs, tree)
-    Block(defs.toList map untpd.TypedSplice, expand(tree1))
-  }
-   */

tests/run/i2939.scala

@@ -0,0 +1,35 @@
+import scala.collection.mutable._
+
+class Tag(val name: String, val buffer: Buffer[Tag] = ArrayBuffer()) {
+	def space(n: Int = 0): String = {
+	    s"${" " * n}<$name>\n" +
+	        (if(buffer.isEmpty) "" else buffer.map(_.space(n + 4)).mkString("", "\n", "\n")) +
+	    s"${" " * n}</$name>"
+	}
+
+	def apply[U](f: implicit Tag => U)(implicit tag: Tag = null): this.type = {
+		f(this)
+		if(tag != null) tag.buffer += this
+		this
+	}
+
+	override def toString(): String = space(0)
+}
+
+object Tag {
+	implicit def toTag(symbol: Symbol): Tag = new Tag(symbol.name)
+
+	def main(args: Array[String]): Unit = {
+	}
+}
+
+
+object Test {
+  def foo(x: Int => Int)(y: Int = 0) = {}
+  def bar(x: => Int)(y: Int = 0) = {}
+
+  def main(args: Array[String]): Unit = {
+    foo(x => x)()
+    bar(args.length)()
+  }
+}