From 863639c9abd32773511c10ac732864337e1bdebe Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 18 Jun 2018 12:23:28 +0200 Subject: [PATCH] Revert changes to data class equals/hashCode (KT-12330) This looked like a small and useful change, but caused so many issues (KT-24474, KT-24790, 30b9caea, and another unreported one -- see the test update in this commit) that it didn't pay off after all. The optimization is not that critical for now, as it's only relevant for data classes where component types have trivial equals/hashCode implementation, which is not very often #KT-12330 Declined --- .../org/jetbrains/kotlin/codegen/AsmUtil.java | 2 +- .../codegen/ImplementationBodyCodegen.java | 32 +++---------------- ...kt => typeParameterWithNonTrivialBound.kt} | 7 ++++ .../bytecodeText/dataClasses/kt12330.kt | 4 --- .../ir/IrBlackBoxCodegenTestGenerated.java | 6 ++-- .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++-- .../codegen/BytecodeTextTestGenerated.java | 18 ----------- .../LightAnalysisModeTestGenerated.java | 6 ++-- 8 files changed, 21 insertions(+), 60 deletions(-) rename compiler/testData/codegen/box/dataClasses/{typeParameterWithInterfaceBound.kt => typeParameterWithNonTrivialBound.kt} (75%) delete mode 100644 compiler/testData/codegen/bytecodeText/dataClasses/kt12330.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java index 2dcd9999e7b..93d17e6d461 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java @@ -524,7 +524,7 @@ public class AsmUtil { } } else if (type.getSort() == Type.OBJECT) { - iv.invokevirtual(type.getInternalName(), "hashCode", "()I", false); + iv.invokevirtual("java/lang/Object", "hashCode", "()I", false); } else if (type.getSort() == Type.BOOLEAN) { Label end = new Label(); diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index 5b8e3c5af62..f0d702ef933 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -41,7 +41,6 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; import org.jetbrains.kotlin.resolve.calls.model.VariableAsFunctionResolvedCall; import org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilsKt; -import org.jetbrains.kotlin.resolve.jvm.AsmTypes; import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin; import org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOriginKt; import org.jetbrains.kotlin.resolve.jvm.jvmSignature.JvmClassSignature; @@ -52,7 +51,6 @@ import org.jetbrains.kotlin.resolve.scopes.receivers.ExtensionReceiver; import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitReceiver; import org.jetbrains.kotlin.resolve.scopes.receivers.ReceiverValue; import org.jetbrains.kotlin.types.KotlinType; -import org.jetbrains.kotlin.types.TypeUtils; import org.jetbrains.org.objectweb.asm.FieldVisitor; import org.jetbrains.org.objectweb.asm.Label; import org.jetbrains.org.objectweb.asm.MethodVisitor; @@ -575,23 +573,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { iv.ifne(ne); } else { - if (isPrimitive(asmType)) { - StackValue value = StackValue.cmp(KtTokens.EQEQ, asmType, StackValue.onStack(asmType), StackValue.onStack(asmType)); - value.put(Type.BOOLEAN_TYPE, iv); - } - else { - if (TypeUtils.isNullableType(propertyDescriptor.getType())) { - StackValue value = - genEqualsForExpressionsOnStack(KtTokens.EQEQ, StackValue.onStack(asmType), StackValue.onStack(asmType)); - value.put(Type.BOOLEAN_TYPE, iv); - } - else { - iv.invokevirtual( - getEqualsOrHashCodeOwner(propertyDescriptor).getInternalName(), "equals", - Type.getMethodDescriptor(Type.BOOLEAN_TYPE, AsmTypes.OBJECT_TYPE), false - ); - } - } + StackValue value = + genEqualsForExpressionsOnStack(KtTokens.EQEQ, StackValue.onStack(asmType), StackValue.onStack(asmType)); + value.put(Type.BOOLEAN_TYPE, iv); iv.ifeq(ne); } } @@ -632,7 +616,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { iv.ifnull(ifNull); } - genHashCode(mv, iv, getEqualsOrHashCodeOwner(propertyDescriptor), state.getTarget()); + genHashCode(mv, iv, asmType, state.getTarget()); if (ifNull != null) { Label end = new Label(); @@ -656,14 +640,6 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { FunctionCodegen.endVisit(mv, "hashCode", myClass); } - @NotNull - private Type getEqualsOrHashCodeOwner(@NotNull PropertyDescriptor descriptor) { - ClassifierDescriptor classifier = descriptor.getType().getConstructor().getDeclarationDescriptor(); - return !(classifier instanceof ClassDescriptor) || JvmCodegenUtil.isJvmInterface(classifier) - ? AsmTypes.OBJECT_TYPE - : typeMapper.mapType(descriptor); - } - @Override public void generateToStringMethod(@NotNull FunctionDescriptor function, @NotNull List properties) { MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(function); diff --git a/compiler/testData/codegen/box/dataClasses/typeParameterWithInterfaceBound.kt b/compiler/testData/codegen/box/dataClasses/typeParameterWithNonTrivialBound.kt similarity index 75% rename from compiler/testData/codegen/box/dataClasses/typeParameterWithInterfaceBound.kt rename to compiler/testData/codegen/box/dataClasses/typeParameterWithNonTrivialBound.kt index 2969cf1414a..c7577664ba1 100644 --- a/compiler/testData/codegen/box/dataClasses/typeParameterWithInterfaceBound.kt +++ b/compiler/testData/codegen/box/dataClasses/typeParameterWithNonTrivialBound.kt @@ -10,6 +10,8 @@ annotation class Anno @Anno data class C(val a: Anno) +data class D(val t: T) + fun box(): String { val b1 = B(object : A {}) val b2 = B(object : A {}) @@ -22,5 +24,10 @@ fun box(): String { if (c1.hashCode() != c2.hashCode()) return "Fail 3" if (!c1.equals(c2)) return "Fail 4" + val d1 = D(1) + val d2 = D(2) + if (d1.hashCode() == d2.hashCode()) return "Fail 5" + if (d1.equals(d2)) return "Fail 6" + return "OK" } diff --git a/compiler/testData/codegen/bytecodeText/dataClasses/kt12330.kt b/compiler/testData/codegen/bytecodeText/dataClasses/kt12330.kt deleted file mode 100644 index 42a675886e7..00000000000 --- a/compiler/testData/codegen/bytecodeText/dataClasses/kt12330.kt +++ /dev/null @@ -1,4 +0,0 @@ -data class D(val x: List) - -// 1 INVOKEVIRTUAL java/lang/Object.hashCode -// 1 INVOKEVIRTUAL java/lang/Object.equals \ No newline at end of file diff --git a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index 7c6beccd723..08172f9d17a 100644 --- a/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-ir-jvm/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -7849,9 +7849,9 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/dataClasses/twoVarParams.kt"); } - @TestMetadata("typeParameterWithInterfaceBound.kt") - public void testTypeParameterWithInterfaceBound() throws Exception { - runTest("compiler/testData/codegen/box/dataClasses/typeParameterWithInterfaceBound.kt"); + @TestMetadata("typeParameterWithNonTrivialBound.kt") + public void testTypeParameterWithNonTrivialBound() throws Exception { + runTest("compiler/testData/codegen/box/dataClasses/typeParameterWithNonTrivialBound.kt"); } @TestMetadata("unitComponent.kt") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index f6e5565f7bf..b5974e2aca6 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -7849,9 +7849,9 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/dataClasses/twoVarParams.kt"); } - @TestMetadata("typeParameterWithInterfaceBound.kt") - public void testTypeParameterWithInterfaceBound() throws Exception { - runTest("compiler/testData/codegen/box/dataClasses/typeParameterWithInterfaceBound.kt"); + @TestMetadata("typeParameterWithNonTrivialBound.kt") + public void testTypeParameterWithNonTrivialBound() throws Exception { + runTest("compiler/testData/codegen/box/dataClasses/typeParameterWithNonTrivialBound.kt"); } @TestMetadata("unitComponent.kt") diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java index 6cfcc0d6491..511fc528d25 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeTextTestGenerated.java @@ -1243,24 +1243,6 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest { } } - @TestMetadata("compiler/testData/codegen/bytecodeText/dataClasses") - @TestDataPath("$PROJECT_ROOT") - @RunWith(JUnit3RunnerWithInners.class) - public static class DataClasses extends AbstractBytecodeTextTest { - private void runTest(String testDataFilePath) throws Exception { - KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath); - } - - public void testAllFilesPresentInDataClasses() throws Exception { - KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/dataClasses"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true); - } - - @TestMetadata("kt12330.kt") - public void testKt12330() throws Exception { - runTest("compiler/testData/codegen/bytecodeText/dataClasses/kt12330.kt"); - } - } - @TestMetadata("compiler/testData/codegen/bytecodeText/deadCodeElimination") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 6ac0529dd47..05c63c5ec66 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -7849,9 +7849,9 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/dataClasses/twoVarParams.kt"); } - @TestMetadata("typeParameterWithInterfaceBound.kt") - public void testTypeParameterWithInterfaceBound() throws Exception { - runTest("compiler/testData/codegen/box/dataClasses/typeParameterWithInterfaceBound.kt"); + @TestMetadata("typeParameterWithNonTrivialBound.kt") + public void testTypeParameterWithNonTrivialBound() throws Exception { + runTest("compiler/testData/codegen/box/dataClasses/typeParameterWithNonTrivialBound.kt"); } @TestMetadata("unitComponent.kt")