Method to zip value params and args.

I had all the variations of zip/zipped start logging when they being
were given differently lengthed arguments, to find out what all we might
be throwing away. This uncovered at least one bug with free variable
identification, fixed herein, and also solidifed my belief that we
should have a lot less ad hoc zipping of things. No review.

8 files changed, 77 insertions, 9 deletions

diff --git a/src/compiler/scala/reflect/internal/Definitions.scala b/src/compiler/scala/reflect/internal/Definitions.scala
index a37ae9cd4d..7bf646a118 100644
--- a/src/compiler/scala/reflect/internal/Definitions.scala
+++ b/src/compiler/scala/reflect/internal/Definitions.scala
@@ -292,6 +292,7 @@ trait Definitions extends reflect.api.StandardDefinitions {
     def isJavaRepeatedParamType(tp: Type)  = tp.typeSymbol == JavaRepeatedParamClass
     def isRepeatedParamType(tp: Type)      = isScalaRepeatedParamType(tp) || isJavaRepeatedParamType(tp)
 
+    def isJavaVarArgs(params: List[Symbol])  = params.nonEmpty && isJavaRepeatedParamType(params.last.tpe)
     def isScalaVarArgs(params: List[Symbol]) = params.nonEmpty && isScalaRepeatedParamType(params.last.tpe)
     def isVarArgsList(params: List[Symbol])  = params.nonEmpty && isRepeatedParamType(params.last.tpe)
     def isVarArgTypes(formals: List[Type])   = formals.nonEmpty && isRepeatedParamType(formals.last)
diff --git a/src/compiler/scala/reflect/internal/SymbolTable.scala b/src/compiler/scala/reflect/internal/SymbolTable.scala
index c54b1c43b2..a7592cc81b 100644
--- a/src/compiler/scala/reflect/internal/SymbolTable.scala
+++ b/src/compiler/scala/reflect/internal/SymbolTable.scala
@@ -33,7 +33,8 @@ abstract class SymbolTable extends api.Universe
   def abort(): Nothing = throw new Error()
 
   /** Override with final implementation for inlining. */
-  def debuglog(msg: => String): Unit = if (settings.debug.value) log(msg)
+  def debuglog(msg:  => String): Unit = if (settings.debug.value) log(msg)
+  def debugwarn(msg: => String): Unit = if (settings.debug.value) Console.println(msg)
 
   /** Are we compiling for Java SE? */
   // def forJVM: Boolean
diff --git a/src/compiler/scala/reflect/internal/Symbols.scala b/src/compiler/scala/reflect/internal/Symbols.scala
index 60d544fe17..9757297e8f 100644
--- a/src/compiler/scala/reflect/internal/Symbols.scala
+++ b/src/compiler/scala/reflect/internal/Symbols.scala
@@ -871,7 +871,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
     }
     catch {
       case ex: CyclicReference =>
-        if (settings.debug.value) println("... trying to complete "+this)
+        debugwarn("... hit cycle trying to complete " + this.fullLocationString)
         throw ex
     }
 
diff --git a/src/compiler/scala/reflect/internal/TreeInfo.scala b/src/compiler/scala/reflect/internal/TreeInfo.scala
index de46069ee2..248cee8de8 100644
--- a/src/compiler/scala/reflect/internal/TreeInfo.scala
+++ b/src/compiler/scala/reflect/internal/TreeInfo.scala
@@ -15,9 +15,9 @@ import Flags._
  */
 abstract class TreeInfo {
   val global: SymbolTable
-  import global._
 
-  import definitions.ThrowableClass
+  import global._
+  import definitions.{ isVarArgsList, ThrowableClass }
 
   /* Does not seem to be used. Not sure what it does anyway.
   def isOwnerDefinition(tree: Tree): Boolean = tree match {
@@ -104,6 +104,50 @@ abstract class TreeInfo {
       false
   }
 
+
+  /**
+   * Selects the correct parameter list when there are nested applications.
+   * Given Apply(fn, args), args might correspond to any of fn.symbol's parameter
+   * lists.  To choose the correct one before uncurry, we have to unwrap any
+   * applies: for instance Apply(fn @ Apply(Apply(_, _), _), args) implies args
+   * correspond to the third parameter list.
+   *
+   * Also accounts for varargs.
+   */
+  def zipMethodParamsAndArgs(t: Tree): List[(Symbol, Tree)] = t match {
+    case Apply(fn, args) =>
+      val params = fn.symbol.paramss(applyDepth(fn))
+      val plen   = params.length
+      val alen   = args.length
+      def fail() = {
+        global.debugwarn(
+          "Mismatch trying to zip method parameters and argument list:\n" +
+          "  params = " + params + "\n" +
+          "    args = " + args + "\n" +
+          "    tree = " + t
+        )
+        params zip args
+      }
+
+      if (plen == alen) params zip args
+      else if (params.isEmpty) fail
+      else if (isVarArgsList(params)) {
+        val plenInit = plen - 1
+        if (alen == plenInit) {
+          if (alen == 0) Nil        // avoid calling mismatched zip
+          else params.init zip args
+        }
+        else if (alen < plenInit) fail
+        else {
+          val front = params.init zip (args take plenInit)
+          val back  = args drop plenInit map (a => (params.last, a))
+          front ++ back
+        }
+      }
+      else fail
+    case _  => Nil
+  }
+
   /** Is symbol potentially a getter of a variable?
    */
   def mayBeVarGetter(sym: Symbol): Boolean = sym.info match {
@@ -331,6 +375,15 @@ abstract class TreeInfo {
     case _                      => tree
   }
 
+  /** The depth of the nested applies: e.g. Apply(Apply(Apply(_, _), _), _)
+   *  has depth 3.  Continues through type applications (without counting them.)
+   */
+  def applyDepth(tree: Tree): Int = tree match {
+    case Apply(fn, _)           => 1 + applyDepth(fn)
+    case TypeApply(fn, _)       => applyDepth(fn)
+    case AppliedTypeTree(fn, _) => applyDepth(fn)
+    case _                      => 0
+  }
   def firstArgument(tree: Tree): Tree = tree match {
     case Apply(fn, args) =>
       val f = firstArgument(fn)
diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala
index 4bc00c5f43..8ebab43b10 100644
--- a/src/compiler/scala/tools/nsc/Global.scala
+++ b/src/compiler/scala/tools/nsc/Global.scala
@@ -171,6 +171,13 @@ class Global(var currentSettings: Settings, var reporter: Reporter) extends Symb
     if (settings.debug.value && (settings.log containsPhase globalPhase))
       inform("[log " + phase + "] " + msg)
   }
+  // Warnings issued only under -Ydebug.  For messages which should reach
+  // developer ears, but are not adequately actionable by users.
+  @inline final override def debugwarn(msg: => String) {
+    if (settings.debug.value)
+      warning(msg)
+  }
+
   private def elapsedMessage(msg: String, start: Long) =
     msg + " in " + (currentTime - start) + "ms"
 
diff --git a/src/compiler/scala/tools/nsc/transform/LiftCode.scala b/src/compiler/scala/tools/nsc/transform/LiftCode.scala
index 3392a291ac..bc91f7d8da 100644
--- a/src/compiler/scala/tools/nsc/transform/LiftCode.scala
+++ b/src/compiler/scala/tools/nsc/transform/LiftCode.scala
@@ -409,6 +409,9 @@ abstract class LiftCode extends Transform with TypingTransformers {
      *  would only emerge under more complicated conditions such as #3855.
      *  I'll try to figure it all out, but if someone who already knows the
      *  whole story wants to fill it in, that too would be great.
+     *
+     *  XXX I found this had been cut and pasted between LiftCode and UnCurry,
+     *  and seems to be running in both.
      */
     private val freeLocalsTraverser = new Traverser {
       var currentMethod: Symbol = NoSymbol
@@ -430,7 +433,7 @@ abstract class LiftCode extends Transform with TypingTransformers {
         /** A method call with a by-name parameter represents escape. */
         case Apply(fn, args) if fn.symbol.paramss.nonEmpty =>
           traverse(fn)
-          (fn.symbol.paramss.head, args).zipped foreach { (param, arg) =>
+          for ((param, arg) <- treeInfo.zipMethodParamsAndArgs(tree)) {
             if (param.tpe != null && isByNameParamType(param.tpe))
               withEscaping(traverse(arg))
             else
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index d43dfdd3d9..db39eda3b6 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -284,9 +284,9 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
         for (member <- mixinClass.info.decls) {
           if (isConcreteAccessor(member)) {
             if (isOverriddenAccessor(member, clazz.info.baseClasses)) {
-              if (settings.debug.value)
-                println("!!! is overridden val: "+member)
-            } else {
+              debugwarn("!!! is overridden val: "+member.fullLocationString)
+            }
+            else {
               // mixin field accessors
               val member1 = addMember(
                 clazz,
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
index 19a5da6276..2bc988a523 100644
--- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala
+++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala
@@ -745,6 +745,9 @@ abstract class UnCurry extends InfoTransform
      *  would only emerge under more complicated conditions such as #3855.
      *  I'll try to figure it all out, but if someone who already knows the
      *  whole story wants to fill it in, that too would be great.
+     *
+     *  XXX I found this had been cut and pasted between LiftCode and UnCurry,
+     *  and seems to be running in both.
      */
     val freeLocalsTraverser = new Traverser {
       var currentMethod: Symbol = NoSymbol
@@ -766,7 +769,7 @@ abstract class UnCurry extends InfoTransform
         /** A method call with a by-name parameter represents escape. */
         case Apply(fn, args) if fn.symbol.paramss.nonEmpty =>
           traverse(fn)
-          (fn.symbol.paramss.head, args).zipped foreach { (param, arg) =>
+          for ((param, arg) <- treeInfo.zipMethodParamsAndArgs(tree)) {
             if (param.tpe != null && isByNameParamType(param.tpe))
               withEscaping(traverse(arg))
             else