From fb5806f5fba42711879a97645259c91e62e24064 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 20 Oct 2014 15:09:22 +0400 Subject: [PATCH] Fix inheritance from mutable Java collections Don't use CodegenUtil#getDeclaredFunctionByRawSignature because it's incorrect in case of platform types. Instead use JetTypeMapper to find JVM signatures of methods which are callable on the current class #KT-6042 Fixed --- .../codegen/ImplementationBodyCodegen.java | 111 +++++++++--------- ...thodsForClassExtendingMutableCollection.kt | 24 ++++ .../BlackBoxCodegenTestGenerated.java | 6 + 3 files changed, 83 insertions(+), 58 deletions(-) create mode 100644 compiler/testData/codegen/box/builtinStubMethods/noMethodsForClassExtendingMutableCollection.kt diff --git a/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java index 634e67e1612..bde814f81df 100644 --- a/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/jet/codegen/ImplementationBodyCodegen.java @@ -57,7 +57,7 @@ import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodParameterKind; import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodParameterSignature; import org.jetbrains.jet.lang.resolve.java.jvmSignature.JvmMethodSignature; import org.jetbrains.jet.lang.resolve.name.Name; -import org.jetbrains.jet.lang.types.*; +import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.checker.JetTypeChecker; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.lexer.JetTokens; @@ -510,85 +510,80 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } } - private class MethodStubGenerator { - private final Set generatedSignatures = new HashSet(); + @NotNull + private Set collectSignaturesOfExistingNonAbstractMethods() { + Set existingMethodSignatures = new HashSet(); - public void generate( - @NotNull String name, - @NotNull String desc, - @NotNull ClassifierDescriptor returnedClassifier, - @NotNull ClassifierDescriptor... valueParameterClassifiers - ) { - // avoid generating same signature twice - if (!generatedSignatures.add(name + desc)) return; - if (CodegenUtil.getDeclaredFunctionByRawSignature( - descriptor, Name.identifier(name), returnedClassifier, valueParameterClassifiers) == null) { - int access = descriptor.getKind() == ClassKind.TRAIT ? - ACC_PUBLIC | ACC_ABSTRACT : - ACC_PUBLIC; - MethodVisitor mv = v.newMethod(NO_ORIGIN, access, name, desc, null, null); - if (descriptor.getKind() != ClassKind.TRAIT) { - mv.visitCode(); - genThrow(new InstructionAdapter(mv), "java/lang/UnsupportedOperationException", "Mutating immutable collection"); - FunctionCodegen.endVisit(mv, "built-in stub for " + name + desc, null); - } + for (DeclarationDescriptor member : descriptor.getDefaultType().getMemberScope().getAllDescriptors()) { + if (!(member instanceof FunctionDescriptor)) continue; + FunctionDescriptor function = (FunctionDescriptor) member; + if (function.getModality() == Modality.ABSTRACT) continue; + for (FunctionDescriptor overridden : DescriptorUtils.getAllOverriddenDescriptors(function)) { + Method overriddenMethod = typeMapper.mapSignature(overridden.getOriginal()).getAsmMethod(); + existingMethodSignatures.add(overriddenMethod.getName() + overriddenMethod.getDescriptor()); + } + + Method method = typeMapper.mapSignature(function.getOriginal()).getAsmMethod(); + existingMethodSignatures.add(method.getName() + method.getDescriptor()); + } + + return existingMethodSignatures; + } + + private void generateMethodStubs(@NotNull Set signatures) { + if (signatures.isEmpty()) return; + + Set existingMethodSignatures = collectSignaturesOfExistingNonAbstractMethods(); + + for (String signature : signatures) { + if (!existingMethodSignatures.add(signature)) return; + int access = descriptor.getKind() == ClassKind.TRAIT ? + ACC_PUBLIC | ACC_ABSTRACT : + ACC_PUBLIC; + int paren = signature.indexOf('('); + MethodVisitor mv = v.newMethod(NO_ORIGIN, access, signature.substring(0, paren), signature.substring(paren), null, null); + if (descriptor.getKind() != ClassKind.TRAIT) { + mv.visitCode(); + genThrow(new InstructionAdapter(mv), "java/lang/UnsupportedOperationException", "Mutating immutable collection"); + FunctionCodegen.endVisit(mv, "built-in stub for " + signature, null); } } } private void generateBuiltinMethodStubs() { KotlinBuiltIns builtIns = KotlinBuiltIns.getInstance(); - MethodStubGenerator methodStubs = new MethodStubGenerator(); + Set methodStubs = new LinkedHashSet(); if (isSubclass(descriptor, builtIns.getCollection())) { - ClassifierDescriptor classifier = getSubstituteForTypeParameterOf(builtIns.getCollection(), 0); - - methodStubs.generate("add", "(Ljava/lang/Object;)Z", builtIns.getBoolean(), classifier); - methodStubs.generate("remove", "(Ljava/lang/Object;)Z", builtIns.getBoolean(), builtIns.getAny()); - methodStubs.generate("addAll", "(Ljava/util/Collection;)Z", builtIns.getBoolean(), builtIns.getCollection()); - methodStubs.generate("removeAll", "(Ljava/util/Collection;)Z", builtIns.getBoolean(), builtIns.getCollection()); - methodStubs.generate("retainAll", "(Ljava/util/Collection;)Z", builtIns.getBoolean(), builtIns.getCollection()); - methodStubs.generate("clear", "()V", builtIns.getUnit()); + methodStubs.add("add(Ljava/lang/Object;)Z"); + methodStubs.add("remove(Ljava/lang/Object;)Z"); + methodStubs.add("addAll(Ljava/util/Collection;)Z"); + methodStubs.add("removeAll(Ljava/util/Collection;)Z"); + methodStubs.add("retainAll(Ljava/util/Collection;)Z"); + methodStubs.add("clear()V"); } if (isSubclass(descriptor, builtIns.getList())) { - ClassifierDescriptor classifier = getSubstituteForTypeParameterOf(builtIns.getList(), 0); - - methodStubs.generate("set", "(ILjava/lang/Object;)Ljava/lang/Object;", classifier, builtIns.getInt(), classifier); - methodStubs.generate("add", "(ILjava/lang/Object;)V", builtIns.getUnit(), builtIns.getInt(), classifier); - methodStubs.generate("remove", "(I)Ljava/lang/Object;", classifier, builtIns.getInt()); + methodStubs.add("set(ILjava/lang/Object;)Ljava/lang/Object;"); + methodStubs.add("add(ILjava/lang/Object;)V"); + methodStubs.add("remove(I)Ljava/lang/Object;"); } if (isSubclass(descriptor, builtIns.getMap())) { - ClassifierDescriptor keyClassifier = getSubstituteForTypeParameterOf(builtIns.getMap(), 0); - ClassifierDescriptor valueClassifier = getSubstituteForTypeParameterOf(builtIns.getMap(), 1); - - methodStubs.generate("put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;", valueClassifier, keyClassifier, - valueClassifier); - methodStubs.generate("remove", "(Ljava/lang/Object;)Ljava/lang/Object;", valueClassifier, builtIns.getAny()); - methodStubs.generate("putAll", "(Ljava/util/Map;)V", builtIns.getUnit(), builtIns.getMap()); - methodStubs.generate("clear", "()V", builtIns.getUnit()); + methodStubs.add("put(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"); + methodStubs.add("remove(Ljava/lang/Object;)Ljava/lang/Object;"); + methodStubs.add("putAll(Ljava/util/Map;)V"); + methodStubs.add("clear()V"); } if (isSubclass(descriptor, builtIns.getMapEntry())) { - ClassifierDescriptor valueClassifier = getSubstituteForTypeParameterOf(builtIns.getMapEntry(), 1); - - methodStubs.generate("setValue", "(Ljava/lang/Object;)Ljava/lang/Object;", valueClassifier, valueClassifier); + methodStubs.add("setValue(Ljava/lang/Object;)Ljava/lang/Object;"); } if (isSubclass(descriptor, builtIns.getIterator())) { - methodStubs.generate("remove", "()V", builtIns.getUnit()); + methodStubs.add("remove()V"); } - } - @NotNull - private ClassifierDescriptor getSubstituteForTypeParameterOf(@NotNull ClassDescriptor trait, int index) { - TypeParameterDescriptor listTypeParameter = trait.getTypeConstructor().getParameters().get(index); - TypeSubstitutor deepSubstitutor = SubstitutionUtils.buildDeepSubstitutor(descriptor.getDefaultType()); - TypeProjection substitute = deepSubstitutor.substitute(new TypeProjectionImpl(listTypeParameter.getDefaultType())); - assert substitute != null : "Couldn't substitute: " + descriptor; - ClassifierDescriptor classifier = substitute.getType().getConstructor().getDeclarationDescriptor(); - assert classifier != null : "No classifier: " + substitute.getType(); - return classifier; + generateMethodStubs(methodStubs); } private void generateFunctionsForDataClasses() { diff --git a/compiler/testData/codegen/box/builtinStubMethods/noMethodsForClassExtendingMutableCollection.kt b/compiler/testData/codegen/box/builtinStubMethods/noMethodsForClassExtendingMutableCollection.kt new file mode 100644 index 00000000000..255cbbaf192 --- /dev/null +++ b/compiler/testData/codegen/box/builtinStubMethods/noMethodsForClassExtendingMutableCollection.kt @@ -0,0 +1,24 @@ +// KT-6042 java.lang.UnsupportedOperationException with ArrayList + +import java.util.ArrayList + +class A : ArrayList() + +fun box(): String { + val a = A() + val b = A() + + a.addAll(b) + a.addAll(0, b) + a.removeAll(b) + a.retainAll(b) + a.clear() + + a.add("") + a.set(0, "") + a.add(0, "") + a.remove(0) + a.remove("") + + return "OK" +} diff --git a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java index 29b80f11d10..c461e7885f4 100644 --- a/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -742,6 +742,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("noMethodsForClassExtendingMutableCollection.kt") + public void testNoMethodsForClassExtendingMutableCollection() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/builtinStubMethods/noMethodsForClassExtendingMutableCollection.kt"); + doTest(fileName); + } + @TestMetadata("SubstitutedList.kt") public void testSubstitutedList() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/codegen/box/builtinStubMethods/SubstitutedList.kt");