Optimization to string concatenation.

with a primitive CONCAT and several opcodes, I'm assuming nobody will
oppose this on principle. Given an expression of the exact form "" + x
(that is, the NPE-immune and much less ugly way to call .toString) we
used to get 4x the bytecode and create an unneeded StringBuilder. Now
this:

0:	aload_1
1:	invokestatic	#20; //Method java/lang/String.valueOf:(Ljava/lang/Object;)Ljava/lang/String;
4:	areturn

Closes #4160, review by dragos.

3 files changed, 63 insertions, 16 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 7bfabc239f..e9d0b6c90e 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -26,7 +26,7 @@ abstract class GenICode extends SubComponent  {
   import icodes._
   import icodes.opcodes._
   import definitions.{
-    ArrayClass, ObjectClass, ThrowableClass, StringClass, NothingClass, NullClass, AnyRefClass,
+    ArrayClass, ObjectClass, ThrowableClass, StringClass, StringModule, NothingClass, NullClass, AnyRefClass,
     Object_equals, Object_isInstanceOf, Object_asInstanceOf, ScalaRunTimeModule,
     BoxedNumberClass, BoxedCharacterClass,
     getMember, runtimeCompanions
@@ -1321,6 +1321,34 @@ abstract class GenICode extends SubComponent  {
       }
     }
 
+    /** The Object => String overload.
+     */
+    private lazy val String_valueOf: Symbol = getMember(StringModule, "valueOf") filter (sym =>
+      sym.info.paramTypes match {
+        case List(pt) => pt.typeSymbol == ObjectClass
+        case _        => false
+      }
+    )
+
+    // I wrote it this way before I realized all the primitive types are
+    // boxed at this point, so I'd have to unbox them.  Keeping it around in
+    // case we want to get more precise.
+    //
+    // private def valueOfForType(tp: Type): Symbol = {
+    //   val xs = getMember(StringModule, "valueOf") filter (sym =>
+    //     // We always exclude the Array[Char] overload because java throws an NPE if
+    //     // you pass it a null.  It will instead find the Object one, which doesn't.
+    //     sym.info.paramTypes match {
+    //       case List(pt) => pt.typeSymbol != ArrayClass && (tp <:< pt)
+    //       case _        => false
+    //     }
+    //   )
+    //   xs.alternatives match {
+    //     case List(sym)  => sym
+    //     case _          => NoSymbol
+    //   }
+    // }
+
     /** Generate string concatenation.
      *
      *  @param tree ...
@@ -1328,22 +1356,25 @@ abstract class GenICode extends SubComponent  {
      *  @return     ...
      */
     def genStringConcat(tree: Tree, ctx: Context): Context = {
-      val Apply(Select(larg, _), rarg) = tree
-      var ctx1 = ctx
-
-      val concatenations = liftStringConcat(tree)
-      if (settings.debug.value)
-        log("Lifted string concatenations for " + tree + "\n to: " + concatenations);
-
-      ctx1.bb.emit(CALL_PRIMITIVE(StartConcat), tree.pos);
-      for (elem <- concatenations) {
-        val kind = toTypeKind(elem.tpe)
-        ctx1 = genLoad(elem, ctx1, kind)
-        ctx1.bb.emit(CALL_PRIMITIVE(StringConcat(kind)), elem.pos)
+      liftStringConcat(tree) match {
+        // Optimization for expressions of the form "" + x.  We can avoid the StringBuilder.
+        case List(Literal(Constant("")), arg) =>
+          if (settings.debug.value) log("Rewriting \"\" + x as String.valueOf(x) for: " + arg)
+          val ctx1 = genLoad(arg, ctx, ObjectReference)
+          ctx1.bb.emit(CALL_METHOD(String_valueOf, Static(false)), arg.pos)
+          ctx1
+        case concatenations =>
+          if (settings.debug.value) log("Lifted string concatenations for " + tree + "\n to: " + concatenations)
+          var ctx1 = ctx
+          ctx1.bb.emit(CALL_PRIMITIVE(StartConcat), tree.pos)
+          for (elem <- concatenations) {
+            val kind = toTypeKind(elem.tpe)
+            ctx1 = genLoad(elem, ctx1, kind)
+            ctx1.bb.emit(CALL_PRIMITIVE(StringConcat(kind)), elem.pos)
+          }
+          ctx1.bb.emit(CALL_PRIMITIVE(EndConcat), tree.pos)
+          ctx1
       }
-      ctx1.bb.emit(CALL_PRIMITIVE(EndConcat), tree.pos)
-
-      ctx1
     }
 
     /** Generate the scala ## method.
diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index 26b4f88f0c..659b7e80d2 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -149,6 +149,7 @@ trait Definitions extends reflect.generic.StandardDefinitions {
     lazy val PartialFunctionClass = getClass("scala.PartialFunction")
     lazy val SymbolClass          = getClass("scala.Symbol")
     lazy val StringClass          = getClass(sn.String)
+    lazy val StringModule         = StringClass.linkedClassOfClass
     lazy val ClassClass           = getClass(sn.Class)
       def Class_getMethod = getMember(ClassClass, nme.getMethod_)
 
diff --git a/test/files/run/concat-two-strings.scala b/test/files/run/concat-two-strings.scala
new file mode 100644
index 0000000000..c8881aa146
--- /dev/null
+++ b/test/files/run/concat-two-strings.scala
@@ -0,0 +1,15 @@
+/** This doesn't test that the optimization is working, only that
+ *  nothing is exploding.
+ */
+object Test {
+  def f1(x: AnyRef)      = "" + x
+  def f2(x: Int)         = "" + x
+  def f3(x: Array[Char]) = "" + x
+  def f4(x: List[Int])   = "" + x
+  def f5(x: Any)         = "" + x
+  def f6(x: AnyVal)      = "" + x
+
+  def main(args: Array[String]): Unit = {
+    List(f1("a"), f2(5), f3(null), f3(Array('a')), f4(List(1)), f5(null), f6(55d)) mkString ""
+  }
+}