From a946f787bc3d352d40680251b2f519f3b6406a88 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Wed, 9 Sep 2015 18:10:51 +0300 Subject: [PATCH] Fix exception from data class codegen for light classes EA-66827 --- .../kotlin/backend/common/CodegenUtil.java | 33 --------- .../common/DataClassMethodGenerator.java | 69 +++++++++---------- .../codegen/ImplementationBodyCodegen.java | 40 +++-------- .../ea66827_dataClassWrongToString.kt | 7 ++ .../ea66827_dataClassWrongToString.txt | 12 ++++ ...JetDiagnosticsTestWithStdLibGenerated.java | 6 ++ .../declaration/JsDataClassGenerator.java | 19 ++--- 7 files changed, 74 insertions(+), 112 deletions(-) create mode 100644 compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.kt create mode 100644 compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.txt diff --git a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.java b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.java index 4b5dc88d1c9..91ad11a8366 100644 --- a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.java +++ b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.java @@ -19,11 +19,9 @@ package org.jetbrains.kotlin.backend.common; import com.intellij.openapi.editor.Document; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiFile; -import kotlin.KotlinPackage; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.backend.common.bridges.BridgesPackage; -import org.jetbrains.kotlin.builtins.KotlinBuiltIns; import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.incremental.components.NoLookupLocation; import org.jetbrains.kotlin.name.Name; @@ -41,19 +39,11 @@ import java.util.*; import static org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilPackage.getBuiltIns; -/** - * Backend-independent utility class. - */ public class CodegenUtil { private CodegenUtil() { } - // TODO: consider putting predefined method signatures here too. - public static final String EQUALS_METHOD_NAME = "equals"; - public static final String TO_STRING_METHOD_NAME = "toString"; - public static final String HASH_CODE_METHOD_NAME = "hashCode"; - @Nullable public static FunctionDescriptor getDeclaredFunctionByRawSignature( @NotNull ClassDescriptor owner, @@ -73,29 +63,6 @@ public class CodegenUtil { return null; } - public static FunctionDescriptor getAnyEqualsMethod(@NotNull KotlinBuiltIns builtIns) { - ClassDescriptor anyClass = builtIns.getAny(); - FunctionDescriptor function = getDeclaredFunctionByRawSignature( - anyClass, Name.identifier(EQUALS_METHOD_NAME), builtIns.getBoolean(), anyClass - ); - assert function != null; - return function; - } - - public static FunctionDescriptor getAnyToStringMethod(@NotNull KotlinBuiltIns builtIns) { - ClassDescriptor anyClass = builtIns.getAny(); - FunctionDescriptor function = getDeclaredFunctionByRawSignature(anyClass, Name.identifier(TO_STRING_METHOD_NAME), builtIns.getString()); - assert function != null; - return function; - } - - public static FunctionDescriptor getAnyHashCodeMethod(@NotNull KotlinBuiltIns builtIns) { - ClassDescriptor anyClass = builtIns.getAny(); - FunctionDescriptor function = getDeclaredFunctionByRawSignature(anyClass, Name.identifier(HASH_CODE_METHOD_NAME), builtIns.getInt()); - assert function != null; - return function; - } - @Nullable public static PropertyDescriptor getDelegatePropertyIfAny(JetExpression expression, ClassDescriptor classDescriptor, BindingContext bindingContext) { PropertyDescriptor propertyDescriptor = null; diff --git a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/DataClassMethodGenerator.java b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/DataClassMethodGenerator.java index f1e1bf770b0..90e53a10512 100644 --- a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/DataClassMethodGenerator.java +++ b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/DataClassMethodGenerator.java @@ -18,6 +18,8 @@ package org.jetbrains.kotlin.backend.common; import com.google.common.collect.Lists; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.kotlin.builtins.KotlinBuiltIns; import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.name.Name; import org.jetbrains.kotlin.psi.JetClass; @@ -41,11 +43,13 @@ public abstract class DataClassMethodGenerator { private final JetClassOrObject declaration; private final BindingContext bindingContext; private final ClassDescriptor classDescriptor; + private final KotlinBuiltIns builtIns; public DataClassMethodGenerator(JetClassOrObject declaration, BindingContext bindingContext) { this.declaration = declaration; this.bindingContext = bindingContext; this.classDescriptor = BindingContextUtils.getNotNull(bindingContext, BindingContext.CLASS, declaration); + this.builtIns = getBuiltIns(classDescriptor); } public void generate() { @@ -61,20 +65,17 @@ public abstract class DataClassMethodGenerator { } } - // Backend-specific implementations. - protected abstract void generateComponentFunction( - @NotNull FunctionDescriptor function, - @NotNull ValueParameterDescriptor parameter - ); + protected abstract void generateComponentFunction(@NotNull FunctionDescriptor function, @NotNull ValueParameterDescriptor parameter); protected abstract void generateCopyFunction(@NotNull FunctionDescriptor function, @NotNull List constructorParameters); - protected abstract void generateToStringMethod(@NotNull List properties); + protected abstract void generateToStringMethod(@NotNull FunctionDescriptor function, @NotNull List properties); - protected abstract void generateHashCodeMethod(@NotNull List properties); + protected abstract void generateHashCodeMethod(@NotNull FunctionDescriptor function, @NotNull List properties); - protected abstract void generateEqualsMethod(@NotNull List properties); + protected abstract void generateEqualsMethod(@NotNull FunctionDescriptor function, @NotNull List properties); + @NotNull protected ClassDescriptor getClassDescriptor() { return classDescriptor; } @@ -101,24 +102,23 @@ public abstract class DataClassMethodGenerator { } private void generateDataClassToStringIfNeeded(@NotNull List properties) { - ClassDescriptor stringClass = getBuiltIns(classDescriptor).getString(); - if (!hasDeclaredNonTrivialMember(CodegenUtil.TO_STRING_METHOD_NAME, stringClass)) { - generateToStringMethod(properties); + FunctionDescriptor function = getDeclaredMember("toString", builtIns.getString()); + if (function != null && isTrivial(function)) { + generateToStringMethod(function, properties); } } private void generateDataClassHashCodeIfNeeded(@NotNull List properties) { - ClassDescriptor intClass = getBuiltIns(classDescriptor).getInt(); - if (!hasDeclaredNonTrivialMember(CodegenUtil.HASH_CODE_METHOD_NAME, intClass)) { - generateHashCodeMethod(properties); + FunctionDescriptor function = getDeclaredMember("hashCode", builtIns.getInt()); + if (function != null && isTrivial(function)) { + generateHashCodeMethod(function, properties); } } private void generateDataClassEqualsIfNeeded(@NotNull List properties) { - ClassDescriptor booleanClass = getBuiltIns(classDescriptor).getBoolean(); - ClassDescriptor anyClass = getBuiltIns(classDescriptor).getAny(); - if (!hasDeclaredNonTrivialMember(CodegenUtil.EQUALS_METHOD_NAME, booleanClass, anyClass)) { - generateEqualsMethod(properties); + FunctionDescriptor function = getDeclaredMember("equals", builtIns.getBoolean(), builtIns.getAny()); + if (function != null && isTrivial(function)) { + generateEqualsMethod(function, properties); } } @@ -132,42 +132,41 @@ public abstract class DataClassMethodGenerator { return result; } - private @NotNull - List getPrimaryConstructorParameters() { + private List getPrimaryConstructorParameters() { if (declaration instanceof JetClass) { - return ((JetClass) declaration).getPrimaryConstructorParameters(); + return declaration.getPrimaryConstructorParameters(); } return Collections.emptyList(); } - /** - * @return true if the class has a declared member with the given name anywhere in its hierarchy besides Any - */ - private boolean hasDeclaredNonTrivialMember( + @Nullable + private FunctionDescriptor getDeclaredMember( @NotNull String name, @NotNull ClassDescriptor returnedClassifier, @NotNull ClassDescriptor... valueParameterClassifiers ) { - FunctionDescriptor function = - CodegenUtil.getDeclaredFunctionByRawSignature(classDescriptor, Name.identifier(name), returnedClassifier, - valueParameterClassifiers); - if (function == null) { - return false; - } + return CodegenUtil.getDeclaredFunctionByRawSignature( + classDescriptor, Name.identifier(name), returnedClassifier, valueParameterClassifiers + ); + } + /** + * @return true if the member is an inherited implementation of a method from Any + */ + private boolean isTrivial(@NotNull FunctionDescriptor function) { if (function.getKind() == CallableMemberDescriptor.Kind.DECLARATION) { - return true; + return false; } for (CallableDescriptor overridden : OverrideResolver.getOverriddenDeclarations(function)) { if (overridden instanceof CallableMemberDescriptor && ((CallableMemberDescriptor) overridden).getKind() == CallableMemberDescriptor.Kind.DECLARATION - && !overridden.getContainingDeclaration().equals(getBuiltIns(classDescriptor).getAny())) { - return true; + && !overridden.getContainingDeclaration().equals(builtIns.getAny())) { + return false; } } - return false; + return true; } } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index 797aca64b16..16d6b75203a 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -484,17 +484,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } @Override - public void generateEqualsMethod(@NotNull List properties) { - KotlinBuiltIns builtins = getBuiltIns(descriptor); - FunctionDescriptor equalsFunction = CodegenUtil.getDeclaredFunctionByRawSignature( - descriptor, Name.identifier(CodegenUtil.EQUALS_METHOD_NAME), builtins.getBoolean(), builtins.getAny() - ); - - assert equalsFunction != null : String.format("Should be called only for classes with non-trivial '%s'. In %s, %s", - CodegenUtil.EQUALS_METHOD_NAME, descriptor.getName(), descriptor); - - MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(equalsFunction); - MethodVisitor mv = v.newMethod(OtherOrigin(equalsFunction), ACC_PUBLIC, "equals", "(Ljava/lang/Object;)Z", null, null); + public void generateEqualsMethod(@NotNull FunctionDescriptor function, @NotNull List properties) { + MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(function); + MethodVisitor mv = v.newMethod(OtherOrigin(function), ACC_PUBLIC, "equals", "(Ljava/lang/Object;)Z", null, null); InstructionAdapter iv = new InstructionAdapter(mv); mv.visitCode(); @@ -560,16 +552,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } @Override - public void generateHashCodeMethod(@NotNull List properties) { - FunctionDescriptor hashCodeFunction = CodegenUtil.getDeclaredFunctionByRawSignature( - descriptor, Name.identifier(CodegenUtil.HASH_CODE_METHOD_NAME), getBuiltIns(descriptor).getInt() - ); - - assert hashCodeFunction != null : String.format("Should be called only for classes with non-trivial '%s'. In %s, %s", - CodegenUtil.HASH_CODE_METHOD_NAME, descriptor.getName(), descriptor); - - MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(hashCodeFunction); - MethodVisitor mv = v.newMethod(OtherOrigin(hashCodeFunction), ACC_PUBLIC, "hashCode", "()I", null, null); + public void generateHashCodeMethod(@NotNull FunctionDescriptor function, @NotNull List properties) { + MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(function); + MethodVisitor mv = v.newMethod(OtherOrigin(function), ACC_PUBLIC, "hashCode", "()I", null, null); InstructionAdapter iv = new InstructionAdapter(mv); mv.visitCode(); @@ -616,16 +601,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } @Override - public void generateToStringMethod(@NotNull List properties) { - FunctionDescriptor toString = CodegenUtil.getDeclaredFunctionByRawSignature( - descriptor, Name.identifier(CodegenUtil.TO_STRING_METHOD_NAME), getBuiltIns(descriptor).getString() - ); - - assert toString != null : String.format("Should be called only for classes with non-trivial '%s'. In %s, %s", - CodegenUtil.TO_STRING_METHOD_NAME, descriptor.getName(), descriptor); - - MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(toString); - MethodVisitor mv = v.newMethod(OtherOrigin(toString), ACC_PUBLIC, "toString", "()Ljava/lang/String;", null, null); + public void generateToStringMethod(@NotNull FunctionDescriptor function, @NotNull List properties) { + MethodContext context = ImplementationBodyCodegen.this.context.intoFunction(function); + MethodVisitor mv = v.newMethod(OtherOrigin(function), ACC_PUBLIC, "toString", "()Ljava/lang/String;", null, null); InstructionAdapter iv = new InstructionAdapter(mv); mv.visitCode(); diff --git a/compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.kt b/compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.kt new file mode 100644 index 00000000000..e39b5976a9b --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.kt @@ -0,0 +1,7 @@ +data class A(val x: Int) { + fun toArray(): IntArray = + intArrayOf(x) + + override fun toString() = + toArray().takeWhile { it != -1 } // .joinToString() +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.txt b/compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.txt new file mode 100644 index 00000000000..a56d5b3870e --- /dev/null +++ b/compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.txt @@ -0,0 +1,12 @@ +package + +kotlin.data() public final class A { + public constructor A(/*0*/ x: kotlin.Int) + public final val x: kotlin.Int + public final /*synthesized*/ fun component1(): kotlin.Int + public final /*synthesized*/ fun copy(/*0*/ x: kotlin.Int = ...): A + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public final fun toArray(): kotlin.IntArray + public open override /*1*/ fun toString(): kotlin.List +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestWithStdLibGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestWithStdLibGenerated.java index a67c16d0753..5f86bea930a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestWithStdLibGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestWithStdLibGenerated.java @@ -763,6 +763,12 @@ public class JetDiagnosticsTestWithStdLibGenerated extends AbstractJetDiagnostic doTest(fileName); } + @TestMetadata("ea66827_dataClassWrongToString.kt") + public void testEa66827_dataClassWrongToString() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/regression/ea66827_dataClassWrongToString.kt"); + doTest(fileName); + } + @TestMetadata("ea70880_illegalJvmName.kt") public void testEa70880_illegalJvmName() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/regression/ea70880_illegalJvmName.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/JsDataClassGenerator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/JsDataClassGenerator.java index 1e7c95ab95b..c63ebd7f674 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/JsDataClassGenerator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/JsDataClassGenerator.java @@ -18,7 +18,6 @@ package org.jetbrains.kotlin.js.translate.declaration; import com.google.dart.compiler.backend.js.ast.*; import org.jetbrains.annotations.NotNull; -import org.jetbrains.kotlin.backend.common.CodegenUtil; import org.jetbrains.kotlin.backend.common.DataClassMethodGenerator; import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.js.translate.context.Namer; @@ -31,9 +30,6 @@ import org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilPackage; import java.util.ArrayList; import java.util.List; -import static kotlin.KotlinPackage.first; -import static org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilPackage.getBuiltIns; - class JsDataClassGenerator extends DataClassMethodGenerator { private final TranslationContext context; private final List output; @@ -91,11 +87,10 @@ class JsDataClassGenerator extends DataClassMethodGenerator { } @Override - public void generateToStringMethod(@NotNull List classProperties) { + public void generateToStringMethod(@NotNull FunctionDescriptor function, @NotNull List classProperties) { // TODO: relax this limitation, with the data generation logic fixed. assert !classProperties.isEmpty(); - FunctionDescriptor prototypeFun = CodegenUtil.getAnyToStringMethod(getBuiltIns(first(classProperties))); - JsFunction functionObj = generateJsMethod(prototypeFun); + JsFunction functionObj = generateJsMethod(function); JsProgram jsProgram = context.program(); JsExpression result = null; @@ -118,9 +113,8 @@ class JsDataClassGenerator extends DataClassMethodGenerator { } @Override - public void generateHashCodeMethod(@NotNull List classProperties) { - FunctionDescriptor prototypeFun = CodegenUtil.getAnyHashCodeMethod(getBuiltIns(first(classProperties))); - JsFunction functionObj = generateJsMethod(prototypeFun); + public void generateHashCodeMethod(@NotNull FunctionDescriptor function, @NotNull List classProperties) { + JsFunction functionObj = generateJsMethod(function); JsProgram jsProgram = context.program(); List statements = functionObj.getBody().getStatements(); @@ -144,10 +138,9 @@ class JsDataClassGenerator extends DataClassMethodGenerator { } @Override - public void generateEqualsMethod(@NotNull List classProperties) { + public void generateEqualsMethod(@NotNull FunctionDescriptor function, @NotNull List classProperties) { assert !classProperties.isEmpty(); - FunctionDescriptor prototypeFun = CodegenUtil.getAnyEqualsMethod(getBuiltIns(first(classProperties))); - JsFunction functionObj = generateJsMethod(prototypeFun); + JsFunction functionObj = generateJsMethod(function); JsFunctionScope funScope = functionObj.getScope(); JsName paramName = funScope.declareName("other");