From 4d69f261e2d4b0e307cb60d8b1ab2b5736cb5e21 Mon Sep 17 00:00:00 2001 From: Simon Ochsenreither Date: Wed, 2 Sep 2015 19:01:37 +0200 Subject: [PATCH 1/3] SI-9315 Desugar string concat to java.lang.StringBuilder ... ... instead of scala.collection.mutable.StringBuilder to benefit from JVM optimizations. Unfortunately primitives are already boxed in erasure when they end up in this part of the backend. Port of cd0211cf0fb755281c7607bf6a172df306b5686f --- .../nsc/backend/jvm/BCodeIdiomatic.scala | 26 +++---- .../tools/nsc/backend/jvm/CoreBTypes.scala | 11 +-- .../scala/reflect/internal/Definitions.scala | 16 +++-- .../reflect/runtime/JavaUniverseForce.scala | 4 +- .../nsc/backend/jvm/StringConcatTest.scala | 70 +++++++++++++++++++ 5 files changed, 102 insertions(+), 25 deletions(-) create mode 100644 test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala index 4bb011adb41c..523ca2acf43f 100644 --- 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 = "" val INSTANCE_CONSTRUCTOR_NAME = "" @@ -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,24 @@ 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) + final def genStringConcat(el: BType, pos: Position): Unit = { + 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 + } - invokevirtual(StringBuilderClassName, "append", bt.descriptor) + val bt = MethodBType(List(jtype), jlStringBuilderRef) + invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor, pos) } /* * can-multi-thread */ final def genEndConcat: Unit = { - invokevirtual(StringBuilderClassName, "toString", "()Ljava/lang/String;") + 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 0b6d2f511907..48061274febc 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala @@ -110,9 +110,9 @@ class CoreBTypes[BTFS <: BTypesFromSymbols[_ <: BackendInterface]](val bTypes: B 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 jlStringBuilderRef : ClassBType = classBTypeFromSymbol(StringBuilderClass) + 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 @@ -240,8 +240,9 @@ final class CoreBTypesProxy[BTFS <: BTypesFromSymbols[_ <: BackendInterface]](va def objArrayReference : ArrayBType = _coreBTypes.objArrayReference def StringReference : ClassBType = _coreBTypes.StringReference - def JavaStringBufferReference : ClassBType = _coreBTypes.JavaStringBufferReference - def JavaCharSequenceReference : ClassBType = _coreBTypes.JavaCharSequenceReference + def jlStringBuilderRef : ClassBType = _coreBTypes.jlStringBuilderRef + def jlStringBufferRef : ClassBType = _coreBTypes.jlStringBufferRef + def jlCharSequenceRef : ClassBType = _coreBTypes.jlCharSequenceRef def StringBuilderReference : ClassBType = _coreBTypes.StringBuilderReference def ThrowableReference : ClassBType = _coreBTypes.ThrowableReference def jlCloneableReference : ClassBType = _coreBTypes.jlCloneableReference diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala index 7e2d1244864d..9462b32c6618 100644 --- 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 index c87b810bdd31..c98d970b9879 100644 --- 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/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala b/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala new file mode 100644 index 000000000000..80cde6c9a940 --- /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", "", "()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) + } +} From 078a866a66bca7a0065dd670a3aaccd32ab47b28 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 26 Oct 2017 14:38:28 +0200 Subject: [PATCH 2/3] Fixup after 4737 --- .../tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 2 +- .../tools/nsc/backend/jvm/BCodeHelpers.scala | 8 ++++---- .../tools/nsc/backend/jvm/BCodeIdiomatic.scala | 16 ++++++++-------- .../tools/nsc/backend/jvm/BackendInterface.scala | 1 + .../scala/tools/nsc/backend/jvm/CoreBTypes.scala | 7 +++---- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index f5f3c4311622..79193f3c3f5a 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -1096,7 +1096,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } - 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 index 6094f4999c55..4a130cdb32e6 100644 --- 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 index 523ca2acf43f..8f580541e45d 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala @@ -222,24 +222,24 @@ trait BCodeIdiomatic { /* * can-multi-thread */ - final def genStringConcat(el: BType, pos: Position): Unit = { + final def genStringConcat(el: BType): Unit = { 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 + case ct: ClassBType if ct.isSubtypeOf(StringRef) => StringRef + case ct: ClassBType if ct.isSubtypeOf(jlStringBufferRef) => jlStringBufferRef + case ct: ClassBType if ct.isSubtypeOf(jlCharSequenceRef) => jlCharSequenceRef + case rt: RefBType => ObjectReference + case pt: PrimitiveBType => pt // Currently this ends up being boxed in erasure } val bt = MethodBType(List(jtype), jlStringBuilderRef) - invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor, pos) + invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor) } /* * can-multi-thread */ final def genEndConcat: Unit = { - invokevirtual(JavaStringBuilderClassName, "toString", "()Ljava/lang/String;", pos) + 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 index bba98ceb5f9a..16c3416003a1 100644 --- 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 index 48061274febc..8f40f230f213 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/CoreBTypes.scala @@ -109,8 +109,8 @@ 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 jlStringBuilderRef : ClassBType = classBTypeFromSymbol(StringBuilderClass) + 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) @@ -239,11 +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 StringRef : ClassBType = _coreBTypes.StringRef def jlStringBuilderRef : ClassBType = _coreBTypes.jlStringBuilderRef def jlStringBufferRef : ClassBType = _coreBTypes.jlStringBufferRef def jlCharSequenceRef : ClassBType = _coreBTypes.jlCharSequenceRef - def StringBuilderReference : ClassBType = _coreBTypes.StringBuilderReference def ThrowableReference : ClassBType = _coreBTypes.ThrowableReference def jlCloneableReference : ClassBType = _coreBTypes.jlCloneableReference def jlNPEReference : ClassBType = _coreBTypes.jlNPEReference From e9a9a431667c4081636106614b7c2631e01a7f90 Mon Sep 17 00:00:00 2001 From: Marko Elezovic Date: Fri, 27 Nov 2015 03:36:27 +0100 Subject: [PATCH 3/3] SI-9571 Avoid boxing primitives in string concatenation Port of cb68d9c1868e9fbb3e58cdfd606274e070db5b31 --- .../nsc/backend/jvm/BCodeBodyBuilder.scala | 14 +- .../nsc/backend/jvm/BCodeIdiomatic.scala | 15 +- test/files/specialized/fft.check | 2 +- .../nsc/backend/jvm/StringConcatTest.scala | 135 +++++++++++++----- 4 files changed, 120 insertions(+), 46 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index 79193f3c3f5a..e17e0d2d01a7 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -1088,9 +1088,17 @@ 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 diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala index 8f580541e45d..eaf45a7bd372 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeIdiomatic.scala @@ -222,16 +222,19 @@ trait BCodeIdiomatic { /* * can-multi-thread */ - final def genStringConcat(el: BType): Unit = { - val jtype = el match { + 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 - case rt: RefBType => ObjectReference - case pt: PrimitiveBType => pt // Currently this ends up being boxed in erasure + // 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(jtype), jlStringBuilderRef) + val bt = MethodBType(List(paramType), jlStringBuilderRef) invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor) } diff --git a/test/files/specialized/fft.check b/test/files/specialized/fft.check index 74cb9bb3b550..5283c6cbe22c 100644 --- 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 index 80cde6c9a940..2a9b8f719810 100644 --- a/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala +++ b/test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala @@ -24,47 +24,110 @@ 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", "", "()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) - } + 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) - @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) - } + def invokeNameDesc(m: String): List[String] = getSingleMethod(c, m).instructions collect { + case Invoke(_, _, name, desc, _) => name + desc + } + assertEquals(invokeNameDesc("t1"), List( + "()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;")) - @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) + assertEquals(invokeNameDesc("t2"), List( + "()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 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) + 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+""", "") + assertEquals(r, "meTTT()trued312312.3-32-4.2sbufchsq//TTTme()trued312312.3-32-4.2sbufchsq") } }