From 9dec0fb779451c31a2a7bcfe17611995276a2d06 Mon Sep 17 00:00:00 2001 From: Laszlo Hornyak Date: Mon, 12 May 2014 21:07:47 +0200 Subject: [PATCH] Correct the argument types of the StringBuilder.append Modified AsmUtil to generate calls to more specific append methods in the StringBuilder class in order to save computation time and make less temporary objects. Also adds unit-test to verify that the append(Object) method was invoked 0 times while the append(String) 3 times Signed-off-by: Laszlo Hornyak --- .../org/jetbrains/jet/codegen/AsmUtil.java | 32 ++++++++++++++++--- .../testData/codegen/bytecodeText/kt5016.kt | 10 ++++++ .../codegen/bytecodeText/kt5016int.kt | 11 +++++++ .../codegen/bytecodeText/kt5016intOrNull.kt | 10 ++++++ .../codegen/BytecodeTextTestGenerated.java | 15 +++++++++ 5 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeText/kt5016.kt create mode 100644 compiler/testData/codegen/bytecodeText/kt5016int.kt create mode 100644 compiler/testData/codegen/bytecodeText/kt5016intOrNull.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java index 60aa5cc8554..8e2418b9070 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/AsmUtil.java @@ -52,6 +52,7 @@ import java.util.Set; import static org.jetbrains.jet.codegen.JvmCodegenUtil.*; import static org.jetbrains.jet.lang.resolve.DescriptorUtils.*; import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.JAVA_STRING_TYPE; +import static org.jetbrains.jet.lang.resolve.java.AsmTypeConstants.getType; import static org.jetbrains.jet.lang.resolve.java.JvmAnnotationNames.ABI_VERSION_FIELD_NAME; import static org.jetbrains.jet.lang.resolve.java.JvmAnnotationNames.KotlinSyntheticClass; import static org.jetbrains.jet.lang.resolve.java.mapping.PrimitiveTypesUtil.asmTypeForPrimitive; @@ -68,6 +69,13 @@ public class AsmUtil { KotlinBuiltIns.getInstance().getChar() ); + private static final Set STRING_BUILDER_OBJECT_APPEND_ARG_TYPES = Sets.newHashSet( + getType(String.class), + getType(StringBuilder.class), + getType(StringBuffer.class), + getType(CharSequence.class) + ); + private static final int NO_FLAG_LOCAL = 0; public static final int NO_FLAG_PACKAGE_PRIVATE = 0; @@ -308,11 +316,25 @@ public class AsmUtil { return type; } - private static Type stringValueOfOrStringBuilderAppendType(Type type) { + private static Type stringValueOfType(Type type) { int sort = type.getSort(); return sort == Type.OBJECT || sort == Type.ARRAY - ? AsmTypeConstants.OBJECT_TYPE - : sort == Type.BYTE || sort == Type.SHORT ? Type.INT_TYPE : type; + ? AsmTypeConstants.OBJECT_TYPE + : sort == Type.BYTE || sort == Type.SHORT ? Type.INT_TYPE : type; + } + + private static Type stringBuilderAppendType(Type type) { + switch (type.getSort()) { + case Type.OBJECT: + return STRING_BUILDER_OBJECT_APPEND_ARG_TYPES.contains(type) ? type : AsmTypeConstants.OBJECT_TYPE; + case Type.ARRAY: + return AsmTypeConstants.OBJECT_TYPE; + case Type.BYTE: + case Type.SHORT: + return Type.INT_TYPE; + default: + return type; + } } public static void genThrow(@NotNull MethodVisitor mv, @NotNull String exception, @NotNull String message) { @@ -374,12 +396,12 @@ public class AsmUtil { } public static void genInvokeAppendMethod(InstructionAdapter v, Type type) { - type = stringValueOfOrStringBuilderAppendType(type); + type = stringBuilderAppendType(type); v.invokevirtual("java/lang/StringBuilder", "append", "(" + type.getDescriptor() + ")Ljava/lang/StringBuilder;"); } public static StackValue genToString(InstructionAdapter v, StackValue receiver, Type receiverType) { - Type type = stringValueOfOrStringBuilderAppendType(receiverType); + Type type = stringValueOfType(receiverType); receiver.put(type, v); v.invokestatic("java/lang/String", "valueOf", "(" + type.getDescriptor() + ")Ljava/lang/String;"); return StackValue.onStack(JAVA_STRING_TYPE); diff --git a/compiler/testData/codegen/bytecodeText/kt5016.kt b/compiler/testData/codegen/bytecodeText/kt5016.kt new file mode 100644 index 00000000000..d8d795e8be1 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/kt5016.kt @@ -0,0 +1,10 @@ +// KT-5016 wrong StringBuilder append method invoked +class kt5016 { + fun f1(name : String) : String { + return "Hello $name!" + } +} + +// 0 INVOKEVIRTUAL java/lang/StringBuilder.append \(Ljava/lang/Object;\)Ljava/lang/StringBuilder +// 3 INVOKEVIRTUAL java/lang/StringBuilder.append \(Ljava/lang/String;\)Ljava/lang/StringBuilder +// 1 INVOKEVIRTUAL java/lang/StringBuilder.toString diff --git a/compiler/testData/codegen/bytecodeText/kt5016int.kt b/compiler/testData/codegen/bytecodeText/kt5016int.kt new file mode 100644 index 00000000000..62727ef5e67 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/kt5016int.kt @@ -0,0 +1,11 @@ +// KT-5016 wrong StringBuilder append method invoked +class kt5016int { + fun f1(num : Int) : String { + return "Hello to all the $num!" + } +} + +// 0 INVOKEVIRTUAL java/lang/StringBuilder.append \(Ljava/lang/Object;\)Ljava/lang/StringBuilder +// 2 INVOKEVIRTUAL java/lang/StringBuilder.append \(Ljava/lang/String;\)Ljava/lang/StringBuilder +// 1 INVOKEVIRTUAL java/lang/StringBuilder.append \(I\)Ljava/lang/StringBuilder +// 1 INVOKEVIRTUAL java/lang/StringBuilder.toString diff --git a/compiler/testData/codegen/bytecodeText/kt5016intOrNull.kt b/compiler/testData/codegen/bytecodeText/kt5016intOrNull.kt new file mode 100644 index 00000000000..fcd1efb4178 --- /dev/null +++ b/compiler/testData/codegen/bytecodeText/kt5016intOrNull.kt @@ -0,0 +1,10 @@ +// KT-5016 wrong StringBuilder append method invoked +class kt5016intOrNull { + fun f1(num : Int?) : String { + return "Hello to all the $num!" + } +} + +// 1 INVOKEVIRTUAL java/lang/StringBuilder.append \(Ljava/lang/Object;\)Ljava/lang/StringBuilder +// 2 INVOKEVIRTUAL java/lang/StringBuilder.append \(Ljava/lang/String;\)Ljava/lang/StringBuilder +// 1 INVOKEVIRTUAL java/lang/StringBuilder.toString diff --git a/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java index a84f13e8581..db6294f8939 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/BytecodeTextTestGenerated.java @@ -107,6 +107,21 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { doTest("compiler/testData/codegen/bytecodeText/kt2887.kt"); } + @TestMetadata("kt5016.kt") + public void testKt5016() throws Exception { + doTest("compiler/testData/codegen/bytecodeText/kt5016.kt"); + } + + @TestMetadata("kt5016int.kt") + public void testKt5016int() throws Exception { + doTest("compiler/testData/codegen/bytecodeText/kt5016int.kt"); + } + + @TestMetadata("kt5016intOrNull.kt") + public void testKt5016intOrNull() throws Exception { + doTest("compiler/testData/codegen/bytecodeText/kt5016intOrNull.kt"); + } + @TestMetadata("noVolatileAnnotation.kt") public void testNoVolatileAnnotation() throws Exception { doTest("compiler/testData/codegen/bytecodeText/noVolatileAnnotation.kt");