From a25416e5cf4d0bd4939318c983766c7b0025a447 Mon Sep 17 00:00:00 2001 From: Alex Tkachman Date: Mon, 2 Jan 2012 09:27:32 +0200 Subject: [PATCH] KT-894 wrong string concatenation --- .../jet/codegen/ExpressionCodegen.java | 2 +- .../jet/codegen/intrinsics/Concat.java | 16 +++--- .../codegen/intrinsics/IntrinsicMethods.java | 3 +- .../jet/codegen/intrinsics/StringPlus.java | 10 +++- .../testData/codegen/regressions/kt894.jet | 8 +++ .../jet/codegen/FunctionGenTest.java | 44 ---------------- .../jetbrains/jet/codegen/StringsTest.java | 51 +++++++++++++++++++ 7 files changed, 79 insertions(+), 55 deletions(-) create mode 100644 compiler/testData/codegen/regressions/kt894.jet diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java index 077c176cfe1..4a5076272ea 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ExpressionCodegen.java @@ -1853,7 +1853,7 @@ public class ExpressionCodegen extends JetVisitor { value.put(lhsType, v); // receiver lhs final IntrinsicMethod intrinsic = (IntrinsicMethod) callable; //noinspection NullableProblems - intrinsic.generate(this, v, lhsType, expression, Arrays.asList(expression.getRight()), null); + intrinsic.generate(this, v, lhsType, expression, Arrays.asList(expression.getRight()), StackValue.onStack(lhsType)); value.store(v); } else { diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/Concat.java b/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/Concat.java index 2e6bda3f6a3..be445388a8f 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/Concat.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/Concat.java @@ -2,6 +2,7 @@ package org.jetbrains.jet.codegen.intrinsics; import com.intellij.psi.PsiElement; import org.jetbrains.jet.codegen.ExpressionCodegen; +import org.jetbrains.jet.codegen.JetTypeMapper; import org.jetbrains.jet.codegen.StackValue; import org.jetbrains.jet.lang.psi.JetExpression; import org.objectweb.asm.Type; @@ -16,17 +17,18 @@ public class Concat implements IntrinsicMethod { @Override public StackValue generate(ExpressionCodegen codegen, InstructionAdapter v, Type expectedType, PsiElement element, List arguments, StackValue receiver) { codegen.generateStringBuilderConstructor(); - if (receiver == null) { // LHS.plus(RHS) + if (receiver == null || receiver == StackValue.none()) { // LHS + RHS + codegen.invokeAppend(arguments.get(0)); // StringBuilder(LHS) + codegen.invokeAppend(arguments.get(1)); + } + else { // LHS.plus(RHS) v.swap(); // StringBuilder LHS codegen.invokeAppendMethod(expectedType); // StringBuilder(LHS) codegen.invokeAppend(arguments.get(0)); } - else { // LHS + RHS - codegen.invokeAppend(arguments.get(0)); // StringBuilder(LHS) - codegen.invokeAppend(arguments.get(1)); - } - + v.invokevirtual("java/lang/StringBuilder", "toString", "()Ljava/lang/String;"); - return StackValue.onStack(Type.getObjectType("java/lang/String")); + StackValue.onStack(JetTypeMapper.JL_STRING_TYPE).put(expectedType, v); + return StackValue.onStack(expectedType); } } diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/IntrinsicMethods.java b/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/IntrinsicMethods.java index 4e1253cbbda..15da00768dd 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/IntrinsicMethods.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/IntrinsicMethods.java @@ -40,6 +40,7 @@ public class IntrinsicMethods { public static final IteratorNext ITERATOR_NEXT = new IteratorNext(); public static final ArraySet ARRAY_SET = new ArraySet(); public static final ArrayGet ARRAY_GET = new ArrayGet(); + public static final StringPlus STRING_PLUS = new StringPlus(); private final Project myProject; private final JetStandardLibrary myStdLib; @@ -87,7 +88,7 @@ public class IntrinsicMethods { declareOverload(myStdLib.getLibraryScope().getFunctions("toString"), 0, new ToString()); declareOverload(myStdLib.getLibraryScope().getFunctions("equals"), 1, EQUALS); declareOverload(myStdLib.getLibraryScope().getFunctions("identityEquals"), 1, EQUALS); - declareOverload(myStdLib.getLibraryScope().getFunctions("plus"), 1, new StringPlus()); + declareOverload(myStdLib.getLibraryScope().getFunctions("plus"), 1, STRING_PLUS); declareOverload(myStdLib.getLibraryScope().getFunctions("Array"), 1, new NewArray()); declareOverload(myStdLib.getLibraryScope().getFunctions("sure"), 0, new Sure()); declareOverload(myStdLib.getLibraryScope().getFunctions("synchronized"), 1, new StupidSync()); diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/StringPlus.java b/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/StringPlus.java index 79995fe4abb..c1f003def72 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/StringPlus.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/intrinsics/StringPlus.java @@ -20,8 +20,14 @@ import java.util.List; public class StringPlus implements IntrinsicMethod { @Override public StackValue generate(ExpressionCodegen codegen, InstructionAdapter v, Type expectedType, PsiElement element, List arguments, StackValue receiver) { - codegen.gen(arguments.get(0)).put(JetTypeMapper.JL_STRING_TYPE, v); - codegen.gen(arguments.get(1)).put(JetTypeMapper.TYPE_OBJECT, v); + if(receiver == null || receiver == StackValue.none()) { + codegen.gen(arguments.get(0)).put(JetTypeMapper.JL_STRING_TYPE, v); + codegen.gen(arguments.get(1)).put(JetTypeMapper.TYPE_OBJECT, v); + } + else { + receiver.put(JetTypeMapper.JL_STRING_TYPE, v); + codegen.gen(arguments.get(0)).put(JetTypeMapper.TYPE_OBJECT, v); + } v.invokestatic("jet/runtime/Intrinsics", "stringPlus", "(Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/String;"); return StackValue.onStack(JetTypeMapper.JL_STRING_TYPE); } diff --git a/compiler/testData/codegen/regressions/kt894.jet b/compiler/testData/codegen/regressions/kt894.jet new file mode 100644 index 00000000000..6cc584b7287 --- /dev/null +++ b/compiler/testData/codegen/regressions/kt894.jet @@ -0,0 +1,8 @@ +fun stringConcat(n : Int) : String? { + var string : String? = "" + for (i in 0..(n - 1)) + string += "LOL " + return string +} + +fun box() = if(stringConcat(3) == "LOL LOL LOL ") "OK" else "fail" \ No newline at end of file diff --git a/compiler/tests/org/jetbrains/jet/codegen/FunctionGenTest.java b/compiler/tests/org/jetbrains/jet/codegen/FunctionGenTest.java index 431400d66bd..79a698d5140 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/FunctionGenTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/FunctionGenTest.java @@ -17,50 +17,6 @@ public class FunctionGenTest extends CodegenTestCase { // System.out.println(generateToText()); } - public void testAnyToString () throws InvocationTargetException, IllegalAccessException { - loadText("fun foo(x: Any) = x.toString()"); -// System.out.println(generateToText()); - Method foo = generateFunction(); - assertEquals("something", foo.invoke(null, "something")); - assertEquals("null", foo.invoke(null, new Object[]{null})); - - } - - public void testNullableAnyToString () throws InvocationTargetException, IllegalAccessException { - loadText("fun foo(x: Any?) = x.toString()"); -// System.out.println(generateToText()); - Method foo = generateFunction(); - assertEquals("something", foo.invoke(null, "something")); - assertEquals("null", foo.invoke(null, new Object[]{null})); - - } - - public void testNullableStringPlus () throws InvocationTargetException, IllegalAccessException { - loadText("fun foo(x: String?, y: Any?) = x + y"); - String text = generateToText(); - assertTrue(text.contains(".stringPlus")); -// System.out.println(text); - Method foo = generateFunction(); - assertEquals("something239", foo.invoke(null, "something", 239)); - assertEquals("null239", foo.invoke(null, null, 239)); - assertEquals("239null", foo.invoke(null, "239", null)); - assertEquals("nullnull", foo.invoke(null, null, null)); - - } - - public void testNonNullableStringPlus () throws InvocationTargetException, IllegalAccessException { - loadText("fun foo(x: String, y: Any?) = x + y + 120"); - String text = generateToText(); - assertFalse(text.contains(".stringPlus")); -// System.out.println(text); - Method foo = generateFunction(); - assertEquals("something239120", foo.invoke(null, "something", 239)); - assertEquals("null239120", foo.invoke(null, null, 239)); - assertEquals("239null120", foo.invoke(null, "239", null)); - assertEquals("nullnull120", foo.invoke(null, null, null)); - - } - public void testAnyEqualsNullable () throws InvocationTargetException, IllegalAccessException { loadText("fun foo(x: Any?) = x.equals(\"lala\")"); // System.out.println(generateToText()); diff --git a/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java b/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java index aa2a0d87277..5998879f623 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java +++ b/compiler/tests/org/jetbrains/jet/codegen/StringsTest.java @@ -1,11 +1,58 @@ package org.jetbrains.jet.codegen; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + /** * @author yole * @author alex.tkachman */ public class StringsTest extends CodegenTestCase { + public void testAnyToString () throws InvocationTargetException, IllegalAccessException { + loadText("fun foo(x: Any) = x.toString()"); +// System.out.println(generateToText()); + Method foo = generateFunction(); + assertEquals("something", foo.invoke(null, "something")); + assertEquals("null", foo.invoke(null, new Object[]{null})); + + } + + public void testNullableAnyToString () throws InvocationTargetException, IllegalAccessException { + loadText("fun foo(x: Any?) = x.toString()"); +// System.out.println(generateToText()); + Method foo = generateFunction(); + assertEquals("something", foo.invoke(null, "something")); + assertEquals("null", foo.invoke(null, new Object[]{null})); + + } + + public void testNullableStringPlus () throws InvocationTargetException, IllegalAccessException { + loadText("fun foo(x: String?, y: Any?) = x + y"); + String text = generateToText(); + assertTrue(text.contains(".stringPlus")); +// System.out.println(text); + Method foo = generateFunction(); + assertEquals("something239", foo.invoke(null, "something", 239)); + assertEquals("null239", foo.invoke(null, null, 239)); + assertEquals("239null", foo.invoke(null, "239", null)); + assertEquals("nullnull", foo.invoke(null, null, null)); + + } + + public void testNonNullableStringPlus () throws InvocationTargetException, IllegalAccessException { + loadText("fun foo(x: String, y: Any?) = x + y + 120"); + String text = generateToText(); + assertFalse(text.contains(".stringPlus")); +// System.out.println(text); + Method foo = generateFunction(); + assertEquals("something239120", foo.invoke(null, "something", 239)); + assertEquals("null239120", foo.invoke(null, null, 239)); + assertEquals("239null120", foo.invoke(null, "239", null)); + assertEquals("nullnull120", foo.invoke(null, null, null)); + + } + public void testRawStrings() throws Exception { blackBoxFile("rawStrings.jet"); } @@ -13,4 +60,8 @@ public class StringsTest extends CodegenTestCase { public void testKt881() throws Exception { blackBoxFile("regressions/kt881.jet"); } + + public void testKt894() throws Exception { + blackBoxFile("regressions/kt894.jet"); + } }