Merge pull request scala#27 from dotty-staging/port-4737

Port scala#4737 and scala#4944
sjrd · Oct 30, 2017 · 9dfe905 · 9dfe905
2 parents 83b68e5 + e9a9a43
commit 9dfe905
Show file tree

Hide file tree

Showing 9 changed files with 189 additions and 38 deletions.
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala
@@ -1088,15 +1088,23 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
         case concatenations =>
           bc.genStartConcat
           for (elem <- concatenations) {
-            val kind = tpeTK(elem)
-            genLoad(elem, kind)
-            bc.genStringConcat(kind)
+            val loadedElem = elem match {
+              case Apply(boxOp, value :: Nil) if isBox(boxOp.symbol) =>
+                // Eliminate boxing of primitive values. Boxing is introduced by erasure because
+                // there's only a single synthetic `+` method "added" to the string class.
+                value
+
+              case _ => elem
+            }
+            val elemType = tpeTK(loadedElem)
+            genLoad(loadedElem, elemType)
+            bc.genConcat(elemType)
           }
           bc.genEndConcat
 
       }
 
-      StringReference
+      StringRef
     }
 
     def genCallMethod(method: Symbol, style: InvokeStyle, hostClass0: Symbol = null, pos: Position = NoPosition): Unit = {

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
@@ -591,7 +591,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
         EMPTY_STRING_ARRAY // no throwable exceptions
       )
 
-      val stringArrayJType: BType = ArrayBType(StringReference)
+      val stringArrayJType: BType = ArrayBType(StringRef)
       val conJType: BType = MethodBType(
         classBTypeFromSymbol(ClassClass) :: stringArrayJType :: stringArrayJType :: Nil,
         UNIT
@@ -604,7 +604,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
           constructor.visitLdcInsn(new java.lang.Integer(fi))
           if (f == null) { constructor.visitInsn(asm.Opcodes.ACONST_NULL) }
           else           { constructor.visitLdcInsn(f) }
-          constructor.visitInsn(StringReference.typedOpcode(asm.Opcodes.IASTORE))
+          constructor.visitInsn(StringRef.typedOpcode(asm.Opcodes.IASTORE))
           fi += 1
         }
       }
@@ -617,12 +617,12 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
 
       // push the string array of field information
       constructor.visitLdcInsn(new java.lang.Integer(fieldList.length))
-      constructor.visitTypeInsn(asm.Opcodes.ANEWARRAY, StringReference.internalName)
+      constructor.visitTypeInsn(asm.Opcodes.ANEWARRAY, StringRef.internalName)
       push(fieldList)
 
       // push the string array of method information
       constructor.visitLdcInsn(new java.lang.Integer(methodList.length))
-      constructor.visitTypeInsn(asm.Opcodes.ANEWARRAY, StringReference.internalName)
+      constructor.visitTypeInsn(asm.Opcodes.ANEWARRAY, StringRef.internalName)
       push(methodList)
 
       // invoke the superclass constructor, which will do the

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala
@@ -43,7 +43,7 @@ trait BCodeIdiomatic {
     if (emitStackMapFrame) asm.ClassWriter.COMPUTE_FRAMES else 0
   )
 
-  val StringBuilderClassName = "scala/collection/mutable/StringBuilder"
+  lazy val JavaStringBuilderClassName = jlStringBuilderRef.internalName
 
   val CLASS_CONSTRUCTOR_NAME    = "<clinit>"
   val INSTANCE_CONSTRUCTOR_NAME = "<init>"
@@ -210,10 +210,10 @@ trait BCodeIdiomatic {
      * can-multi-thread
      */
     final def genStartConcat: Unit = {
-      jmethod.visitTypeInsn(Opcodes.NEW, StringBuilderClassName)
+      jmethod.visitTypeInsn(Opcodes.NEW, JavaStringBuilderClassName)
       jmethod.visitInsn(Opcodes.DUP)
       invokespecial(
-        StringBuilderClassName,
+        JavaStringBuilderClassName,
         INSTANCE_CONSTRUCTOR_NAME,
         "()V"
       )
@@ -222,22 +222,27 @@ trait BCodeIdiomatic {
     /*
      * can-multi-thread
      */
-    final def genStringConcat(el: BType): Unit = {
-
-      val jtype =
-        if (el.isArray || el.isClass) ObjectReference
-        else el
-
-      val bt = MethodBType(List(jtype), StringBuilderReference)
-
-      invokevirtual(StringBuilderClassName, "append", bt.descriptor)
+    def genConcat(elemType: BType): Unit = {
+      val paramType = elemType match {
+        case ct: ClassBType if ct.isSubtypeOf(StringRef)          => StringRef
+        case ct: ClassBType if ct.isSubtypeOf(jlStringBufferRef)  => jlStringBufferRef
+        case ct: ClassBType if ct.isSubtypeOf(jlCharSequenceRef)  => jlCharSequenceRef
+        // Don't match for `ArrayBType(CHAR)`, even though StringBuilder has such an overload:
+        // `"a" + Array('b')` should NOT be "ab", but "a[C@...".
+        case _: RefBType                                              => ObjectReference
+        // jlStringBuilder does not have overloads for byte and short, but we can just use the int version
+        case BYTE | SHORT                                             => INT
+        case pt: PrimitiveBType                                       => pt
+      }
+      val bt = MethodBType(List(paramType), jlStringBuilderRef)
+      invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor)
     }
 
     /*
      * can-multi-thread
      */
     final def genEndConcat: Unit = {
-      invokevirtual(StringBuilderClassName, "toString", "()Ljava/lang/String;")
+      invokevirtual(JavaStringBuilderClassName, "toString", "()Ljava/lang/String;")
     }
 
     /*

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BackendInterface.scala b/src/compiler/scala/tools/nsc/backend/jvm/BackendInterface.scala
@@ -713,6 +713,7 @@ abstract class BackendInterfaceDefinitions { self: BackendInterface =>
   val BoxedDoubleClass: Symbol = requiredClass[java.lang.Double]
   val StringClass: Symbol = requiredClass[java.lang.String]
   val StringBuilderClass: Symbol = requiredClass[scala.collection.mutable.StringBuilder]
+  val JavaStringBuilderClass: Symbol = requiredClass[java.lang.StringBuilder]
   val JavaStringBufferClass: Symbol = requiredClass[java.lang.StringBuffer]
   val JavaCharSequenceClass: Symbol = requiredClass[java.lang.CharSequence]
   val ThrowableClass: Symbol = requiredClass[java.lang.Throwable]

diff --git a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala
@@ -109,10 +109,10 @@ class CoreBTypes[BTFS <: BTypesFromSymbols[_ <: BackendInterface]](val bTypes: B
   lazy val ObjectReference   : ClassBType = classBTypeFromSymbol(ObjectClass)
   lazy val objArrayReference : ArrayBType = ArrayBType(ObjectReference)
 
-  lazy val StringReference             : ClassBType = classBTypeFromSymbol(StringClass)
-  lazy val StringBuilderReference      : ClassBType = classBTypeFromSymbol(StringBuilderClass)
-  lazy val JavaStringBufferReference   : ClassBType = classBTypeFromSymbol(JavaStringBufferClass)
-  lazy val JavaCharSequenceReference   : ClassBType = classBTypeFromSymbol(JavaCharSequenceClass)
+  lazy val StringRef                   : ClassBType = classBTypeFromSymbol(StringClass)
+  lazy val jlStringBuilderRef          : ClassBType = classBTypeFromSymbol(JavaStringBuilderClass)
+  lazy val jlStringBufferRef           : ClassBType = classBTypeFromSymbol(JavaStringBufferClass)
+  lazy val jlCharSequenceRef           : ClassBType = classBTypeFromSymbol(JavaCharSequenceClass)
   lazy val ThrowableReference          : ClassBType = classBTypeFromSymbol(ThrowableClass)
   lazy val jlCloneableReference        : ClassBType = classBTypeFromSymbol(JavaCloneableClass)        // java/lang/Cloneable
   lazy val jlNPEReference              : ClassBType = classBTypeFromSymbol(NullPointerExceptionClass) // java/lang/NullPointerException
@@ -239,10 +239,10 @@ final class CoreBTypesProxy[BTFS <: BTypesFromSymbols[_ <: BackendInterface]](va
   def ObjectReference   : ClassBType = _coreBTypes.ObjectReference
   def objArrayReference : ArrayBType = _coreBTypes.objArrayReference
 
-  def StringReference             : ClassBType = _coreBTypes.StringReference
-  def JavaStringBufferReference   : ClassBType = _coreBTypes.JavaStringBufferReference
-  def JavaCharSequenceReference   : ClassBType = _coreBTypes.JavaCharSequenceReference
-  def StringBuilderReference      : ClassBType = _coreBTypes.StringBuilderReference
+  def StringRef                   : ClassBType = _coreBTypes.StringRef
+  def jlStringBuilderRef          : ClassBType = _coreBTypes.jlStringBuilderRef
+  def jlStringBufferRef           : ClassBType = _coreBTypes.jlStringBufferRef
+  def jlCharSequenceRef           : ClassBType = _coreBTypes.jlCharSequenceRef
   def ThrowableReference          : ClassBType = _coreBTypes.ThrowableReference
   def jlCloneableReference        : ClassBType = _coreBTypes.jlCloneableReference
   def jlNPEReference              : ClassBType = _coreBTypes.jlNPEReference

diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -419,13 +419,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
@@ -263,7 +263,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/files/specialized/fft.check b/test/files/specialized/fft.check
@@ -1,4 +1,4 @@
 Processing 65536 items
 Boxed doubles: 0
-Boxed ints: 2
+Boxed ints: 0
 Boxed longs: 1179811
diff --git a/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala b/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala
@@ -0,0 +1,133 @@
+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
+
+  @Test
+  def appendOverloadNoBoxing(): Unit = {
+    val code =
+      """class C {
+        |  def t1(
+        |         v: Unit,
+        |         z: Boolean,
+        |         c: Char,
+        |         b: Byte,
+        |         s: Short,
+        |         i: Int,
+        |         l: Long,
+        |         f: Float,
+        |         d: Double,
+        |         str: String,
+        |         sbuf: java.lang.StringBuffer,
+        |         chsq: java.lang.CharSequence,
+        |         chrs: Array[Char]) = str + this + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
+        |
+        |  // similar, but starting off with any2stringadd
+        |  def t2(
+        |         v: Unit,
+        |         z: Boolean,
+        |         c: Char,
+        |         b: Byte,
+        |         s: Short,
+        |         i: Int,
+        |         l: Long,
+        |         f: Float,
+        |         d: Double,
+        |         str: String,
+        |         sbuf: java.lang.StringBuffer,
+        |         chsq: java.lang.CharSequence,
+        |         chrs: Array[Char]) = this + str + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
+        |}
+      """.stripMargin
+    val List(c) = compileClasses(compiler)(code)
+
+    def invokeNameDesc(m: String): List[String] = getSingleMethod(c, m).instructions collect {
+      case Invoke(_, _, name, desc, _) => name + desc
+    }
+    assertEquals(invokeNameDesc("t1"), List(
+      "<init>()V",
+      "append(Ljava/lang/String;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/Object;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/Object;)Ljava/lang/StringBuilder;",
+      "append(Z)Ljava/lang/StringBuilder;",
+      "append(C)Ljava/lang/StringBuilder;",
+      "append(I)Ljava/lang/StringBuilder;",
+      "append(I)Ljava/lang/StringBuilder;",
+      "append(I)Ljava/lang/StringBuilder;",
+      "append(F)Ljava/lang/StringBuilder;",
+      "append(J)Ljava/lang/StringBuilder;",
+      "append(D)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/Object;)Ljava/lang/StringBuilder;", // test that we're not using the [C overload
+      "toString()Ljava/lang/String;"))
+
+    assertEquals(invokeNameDesc("t2"), List(
+      "<init>()V",
+      "any2stringadd(Ljava/lang/Object;)Ljava/lang/Object;",
+      "$plus$extension(Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/String;",
+      "append(Ljava/lang/String;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/Object;)Ljava/lang/StringBuilder;",
+      "append(Z)Ljava/lang/StringBuilder;",
+      "append(C)Ljava/lang/StringBuilder;",
+      "append(I)Ljava/lang/StringBuilder;",
+      "append(I)Ljava/lang/StringBuilder;",
+      "append(I)Ljava/lang/StringBuilder;",
+      "append(F)Ljava/lang/StringBuilder;",
+      "append(J)Ljava/lang/StringBuilder;",
+      "append(D)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;",
+      "append(Ljava/lang/Object;)Ljava/lang/StringBuilder;", // test that we're not using the [C overload
+      "toString()Ljava/lang/String;"))
+  }
+
+  @Test
+  def concatPrimitiveCorrectness(): Unit = {
+    val obj: Object = new { override def toString = "TTT" }
+    def t(
+           v: Unit,
+           z: Boolean,
+           c: Char,
+           b: Byte,
+           s: Short,
+           i: Int,
+           l: Long,
+           f: Float,
+           d: Double,
+           str: String,
+           sbuf: java.lang.StringBuffer,
+           chsq: java.lang.CharSequence,
+           chrs: Array[Char]) = {
+      val s1 = str + obj + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
+      val s2 = obj + str + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
+      s1 + "//" + s2
+    }
+    def sbuf = { val r = new java.lang.StringBuffer(); r.append("sbuf"); r }
+    def chsq: java.lang.CharSequence = "chsq"
+    val s = t((), true, 'd', 3: Byte, 12: Short, 3, -32l, 12.3f, -4.2d, "me", sbuf, chsq, Array('a', 'b'))
+    val r = s.replaceAll("""\[C@\w+""", "<ARRAY>")
+    assertEquals(r, "meTTT()trued312312.3-32-4.2sbufchsq<ARRAY>//TTTme()trued312312.3-32-4.2sbufchsq<ARRAY>")
+  }
+}