Merge pull request #4737 from soc/SI-9315

SI-9315 Desugar string concat to java.lang.StringBuilder ...

6 files changed, 101 insertions, 21 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala
index 4a10756468..328a8187c8 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala
@@ -40,7 +40,7 @@ abstract class BCodeIdiomatic extends SubComponent {
     if (emitStackMapFrame) asm.ClassWriter.COMPUTE_FRAMES else 0
   )
 
-  val StringBuilderClassName = "scala/collection/mutable/StringBuilder"
+  lazy val JavaStringBuilderClassName = jlStringBuilderRef.internalName
 
   val EMPTY_STRING_ARRAY   = Array.empty[String]
   val EMPTY_INT_ARRAY      = Array.empty[Int]
@@ -184,10 +184,10 @@ abstract class BCodeIdiomatic extends SubComponent {
      * can-multi-thread
      */
     final def genStartConcat(pos: Position): Unit = {
-      jmethod.visitTypeInsn(Opcodes.NEW, StringBuilderClassName)
+      jmethod.visitTypeInsn(Opcodes.NEW, JavaStringBuilderClassName)
       jmethod.visitInsn(Opcodes.DUP)
       invokespecial(
-        StringBuilderClassName,
+        JavaStringBuilderClassName,
         INSTANCE_CONSTRUCTOR_NAME,
         "()V",
         pos
@@ -198,21 +198,23 @@ abstract class BCodeIdiomatic extends SubComponent {
      * can-multi-thread
      */
     final def genStringConcat(el: BType, pos: Position): Unit = {
-
-      val jtype =
-        if (el.isArray || el.isClass) ObjectRef
-        else el
+      val jtype = el match {
+        case ct: ClassBType if ct.isSubtypeOf(StringRef).get          => StringRef
+        case ct: ClassBType if ct.isSubtypeOf(jlStringBufferRef).get  => jlStringBufferRef
+        case ct: ClassBType if ct.isSubtypeOf(jlCharSequenceRef).get  => jlCharSequenceRef
+        case rt: RefBType                                             => ObjectRef
+        case pt: PrimitiveBType                                       => pt  // Currently this ends up being boxed in erasure
+      }
 
       val bt = MethodBType(List(jtype), jlStringBuilderRef)
-
-      invokevirtual(StringBuilderClassName, "append", bt.descriptor, pos)
+      invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor, pos)
     }
 
     /*
      * can-multi-thread
      */
     final def genEndConcat(pos: Position): Unit = {
-      invokevirtual(StringBuilderClassName, "toString", "()Ljava/lang/String;", pos)
+      invokevirtual(JavaStringBuilderClassName, "toString", "()Ljava/lang/String;", pos)
     }
 
     /*
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala
index 8bb71a386f..6d322d1a34 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala
@@ -95,7 +95,9 @@ class CoreBTypes[BTFS <: BTypesFromSymbols[_ <: Global]](val bTypes: BTFS) {
   lazy val ObjectRef                 : ClassBType = classBTypeFromSymbol(ObjectClass)
   lazy val StringRef                 : ClassBType = classBTypeFromSymbol(StringClass)
   lazy val PredefRef                 : ClassBType = classBTypeFromSymbol(PredefModule.moduleClass)
-  lazy val jlStringBuilderRef        : ClassBType = classBTypeFromSymbol(StringBuilderClass)
+  lazy val jlStringBuilderRef        : ClassBType = classBTypeFromSymbol(JavaStringBuilderClass)
+  lazy val jlStringBufferRef         : ClassBType = classBTypeFromSymbol(JavaStringBufferClass)
+  lazy val jlCharSequenceRef         : ClassBType = classBTypeFromSymbol(JavaCharSequenceClass)
   lazy val jlThrowableRef            : ClassBType = classBTypeFromSymbol(ThrowableClass)
   lazy val jlCloneableRef            : ClassBType = classBTypeFromSymbol(JavaCloneableClass)        // java/lang/Cloneable
   lazy val jiSerializableRef         : ClassBType = classBTypeFromSymbol(JavaSerializableClass)     // java/io/Serializable
@@ -343,6 +345,8 @@ final class CoreBTypesProxy[BTFS <: BTypesFromSymbols[_ <: Global]](val bTypes:
   def StringRef                 : ClassBType = _coreBTypes.StringRef
   def PredefRef                 : ClassBType = _coreBTypes.PredefRef
   def jlStringBuilderRef        : ClassBType = _coreBTypes.jlStringBuilderRef
+  def jlStringBufferRef         : ClassBType = _coreBTypes.jlStringBufferRef
+  def jlCharSequenceRef         : ClassBType = _coreBTypes.jlCharSequenceRef
   def jlThrowableRef            : ClassBType = _coreBTypes.jlThrowableRef
   def jlCloneableRef            : ClassBType = _coreBTypes.jlCloneableRef
   def jiSerializableRef         : ClassBType = _coreBTypes.jiSerializableRef
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/opt/BoxUnbox.scala b/src/compiler/scala/tools/nsc/backend/jvm/opt/BoxUnbox.scala
index 4ecaf17a10..16fe2e5cff 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/opt/BoxUnbox.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/opt/BoxUnbox.scala
@@ -21,7 +21,7 @@ class BoxUnbox[BT <: BTypes](val btypes: BT) {
   import backendUtils._
 
   /**
-   * Eliminate box-unbox paris within `method`. Such appear commonly after closure elimination:
+   * Eliminate box-unbox pairs within `method`. Such appear commonly after closure elimination:
    *
    *   def t2 = {
    *     val f = (b: Byte, i: Int) => i + b // no specialized variant for this function type
@@ -767,7 +767,7 @@ class BoxUnbox[BT <: BTypes](val btypes: BT) {
     private def isSpecializedTupleClass(tupleClass: InternalName) = specializedTupleClassR.pattern.matcher(tupleClass).matches
 
     private val specializedTupleGetterR = "_[12]\\$mc[IJDCZ]\\$sp".r
-    private def isSpecializedTupleGetter(mi: MethodInsnNode) = specializedTupleGetterR.pattern.matcher(mi.name)matches
+    private def isSpecializedTupleGetter(mi: MethodInsnNode) = specializedTupleGetterR.pattern.matcher(mi.name).matches
 
     private val tupleGetterR = "_\\d\\d?".r
     private def isTupleGetter(mi: MethodInsnNode) = tupleGetterR.pattern.matcher(mi.name).matches
diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index ba6c363918..a025407672 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -428,13 +428,15 @@ trait Definitions extends api.StandardDefinitions {
     def elementType(container: Symbol, tp: Type): Type       = elementExtract(container, tp)
 
     // collections classes
-    lazy val ConsClass          = requiredClass[scala.collection.immutable.::[_]]
-    lazy val IteratorClass      = requiredClass[scala.collection.Iterator[_]]
-    lazy val IterableClass      = requiredClass[scala.collection.Iterable[_]]
-    lazy val ListClass          = requiredClass[scala.collection.immutable.List[_]]
-    lazy val SeqClass           = requiredClass[scala.collection.Seq[_]]
-    lazy val StringBuilderClass = requiredClass[scala.collection.mutable.StringBuilder]
-    lazy val TraversableClass   = requiredClass[scala.collection.Traversable[_]]
+    lazy val ConsClass              = requiredClass[scala.collection.immutable.::[_]]
+    lazy val IteratorClass          = requiredClass[scala.collection.Iterator[_]]
+    lazy val IterableClass          = requiredClass[scala.collection.Iterable[_]]
+    lazy val ListClass              = requiredClass[scala.collection.immutable.List[_]]
+    lazy val SeqClass               = requiredClass[scala.collection.Seq[_]]
+    lazy val JavaStringBuilderClass = requiredClass[java.lang.StringBuilder]
+    lazy val JavaStringBufferClass  = requiredClass[java.lang.StringBuffer]
+    lazy val JavaCharSequenceClass  = requiredClass[java.lang.CharSequence]
+    lazy val TraversableClass       = requiredClass[scala.collection.Traversable[_]]
 
     lazy val ListModule       = requiredModule[scala.collection.immutable.List.type]
          def List_apply       = getMemberMethod(ListModule, nme.apply)
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
index d6b611a3f4..e5f77c322d 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
@@ -266,7 +266,9 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.IterableClass
     definitions.ListClass
     definitions.SeqClass
-    definitions.StringBuilderClass
+    definitions.JavaStringBuilderClass
+    definitions.JavaStringBufferClass
+    definitions.JavaCharSequenceClass
     definitions.TraversableClass
     definitions.ListModule
     definitions.NilModule
diff --git a/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala b/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala
new file mode 100644
index 0000000000..80cde6c9a9
--- /dev/null
+++ b/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala
@@ -0,0 +1,70 @@
+package scala.tools.nsc
+package backend.jvm
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import scala.tools.asm.Opcodes._
+import org.junit.Assert._
+
+import scala.tools.testing.AssertUtil._
+
+import CodeGenTools._
+import scala.tools.partest.ASMConverters
+import ASMConverters._
+import scala.tools.testing.ClearAfterClass
+
+object StringConcatTest extends ClearAfterClass.Clearable {
+  var compiler = newCompiler()
+  def clear(): Unit = { compiler = null }
+}
+
+@RunWith(classOf[JUnit4])
+class StringConcatTest extends ClearAfterClass {
+  ClearAfterClass.stateToClear = StringConcatTest
+  val compiler = StringConcatTest.compiler
+
+  val commonPreInstructions = List(Label(0), LineNumber(1, Label(0)), TypeOp(NEW, "java/lang/StringBuilder"), Op(DUP), Invoke(INVOKESPECIAL, "java/lang/StringBuilder", "<init>", "()V", false), Ldc(LDC, "abc"), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/String;)Ljava/lang/StringBuilder;", false), VarOp(ALOAD, 0))
+
+  val commonPostInstructions = List(Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "toString", "()Ljava/lang/String;", false), Op(ARETURN), Label(12))
+
+  def instructionsWithCommonParts(instructions: List[Instruction]) = commonPreInstructions ++ instructions ++ commonPostInstructions
+
+  def instructionsForResultMethod(code: String): List[Instruction] = {
+    val methods = compileMethods(compiler)(code)
+    val resultMethod = methods.find(_.name == "result").get
+    instructionsFromMethod(resultMethod)
+  }
+
+  @Test
+  def concatStringToStringBuilder: Unit = {
+    val code = """ def string = "def"; def result = "abc" + string """
+    val actualInstructions = instructionsForResultMethod(code)
+    val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "string", "()Ljava/lang/String;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/String;)Ljava/lang/StringBuilder;", false)))
+    assertSameCode(actualInstructions, expectedInstructions)
+  }
+
+  @Test
+  def concatStringBufferToStringBuilder: Unit = {
+    val code = """ def stringBuffer = new java.lang.StringBuffer("def"); def result = "abc" + stringBuffer """
+    val actualInstructions = instructionsForResultMethod(code)
+    val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "stringBuffer", "()Ljava/lang/StringBuffer;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder;", false)))
+    assertSameCode(actualInstructions, expectedInstructions)
+  }
+
+  @Test
+  def concatCharSequenceToStringBuilder: Unit = {
+    val code = """ def charSequence: CharSequence = "def"; def result = "abc" + charSequence """
+    val actualInstructions = instructionsForResultMethod(code)
+    val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "charSequence", "()Ljava/lang/CharSequence;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;", false)))
+    assertSameCode(actualInstructions, expectedInstructions)
+  }
+
+  @Test
+  def concatIntToStringBuilder: Unit = {
+    val code = """ def int = 123; def result = "abc" + int """
+    val actualInstructions = instructionsForResultMethod(code)
+    val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "int", "()I", false), Invoke(INVOKESTATIC, "scala/runtime/BoxesRunTime", "boxToInteger", "(I)Ljava/lang/Integer;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/Object;)Ljava/lang/StringBuilder;", false)))
+    assertSameCode(actualInstructions, expectedInstructions)
+  }
+}