From 7c7786f7d058658a14e712e02276761ba2ccb030 Mon Sep 17 00:00:00 2001 From: Michael Bogdanov Date: Fri, 4 Dec 2015 16:12:11 +0300 Subject: [PATCH] Generate private methods in TraitImpl as private, don't generate delegation to private trait methods --- .../kotlin/backend/common/CodegenUtil.java | 4 ++-- .../org/jetbrains/kotlin/codegen/AsmUtil.java | 3 --- .../kotlin/codegen/ClassBodyCodegen.java | 6 ++++++ .../codegen/ImplementationBodyCodegen.java | 9 ++++---- .../codegen/InterfaceImplBodyCodegen.kt | 2 ++ .../kotlin/codegen/MemberCodegen.java | 3 ++- .../codegen/context/CodegenContext.java | 3 ++- .../context/DefaultImplsClassContext.kt | 8 +++++++ .../kotlin/codegen/state/JetTypeMapper.java | 2 +- .../PrivateInTrait.NoCompile.java | 8 +++---- .../codegen/box/traits/noPrivateDelegation.kt | 16 ++++++++++++++ .../codegen/box/traits/syntheticAccessor.kt | 21 +++++++++++++++++++ .../BlackBoxCodegenTestGenerated.java | 12 +++++++++++ .../translate/declaration/ClassTranslator.kt | 2 +- 14 files changed, 81 insertions(+), 18 deletions(-) create mode 100644 compiler/testData/codegen/box/traits/noPrivateDelegation.kt create mode 100644 compiler/testData/codegen/box/traits/syntheticAccessor.kt 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 6bc3e95e586..a86a8307deb 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 @@ -93,14 +93,14 @@ public class CodegenUtil { } @NotNull - public static Map getTraitMethods(ClassDescriptor descriptor) { + public static Map getNonPrivateTraitMethods(ClassDescriptor descriptor) { Map result = new LinkedHashMap(); for (DeclarationDescriptor declaration : DescriptorUtils.getAllDescriptors(descriptor.getDefaultType().getMemberScope())) { if (!(declaration instanceof CallableMemberDescriptor)) continue; CallableMemberDescriptor inheritedMember = (CallableMemberDescriptor) declaration; CallableMemberDescriptor traitMember = ImplKt.findTraitImplementation(inheritedMember); - if (traitMember == null) continue; + if (traitMember == null || Visibilities.isPrivate(traitMember.getVisibility())) continue; assert traitMember.getModality() != Modality.ABSTRACT : "Cannot delegate to abstract trait method: " + inheritedMember; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java index 52965db96ff..8ad2fcc10db 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/AsmUtil.java @@ -325,9 +325,6 @@ public class AsmUtil { private static Integer specialCaseVisibility(@NotNull MemberDescriptor memberDescriptor) { DeclarationDescriptor containingDeclaration = memberDescriptor.getContainingDeclaration(); Visibility memberVisibility = memberDescriptor.getVisibility(); - if (isJvmInterface(containingDeclaration)) { - return memberVisibility == Visibilities.PRIVATE ? NO_FLAG_PACKAGE_PRIVATE : ACC_PUBLIC; - } if (memberVisibility == Visibilities.LOCAL && memberDescriptor instanceof CallableMemberDescriptor) { return ACC_PUBLIC; diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ClassBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ClassBodyCodegen.java index b55a356d278..cb356d99428 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ClassBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ClassBodyCodegen.java @@ -77,6 +77,8 @@ public abstract class ClassBodyCodegen extends MemberCodegen { generatePrimaryConstructorProperties(); generateConstructors(); + generateDefaultImplsIfNeeded(); + for (KtObjectDeclaration companion : companions) { generateDeclaration(companion); } @@ -109,6 +111,10 @@ public abstract class ClassBodyCodegen extends MemberCodegen { } + protected void generateDefaultImplsIfNeeded() { + + } + private static boolean shouldProcessFirst(KtDeclaration declaration) { return !(declaration instanceof KtProperty || declaration instanceof KtNamedFunction); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index a27c29bbbd0..9c2df155b72 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -230,7 +230,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { } @Override - protected void generateBody() { + protected void generateDefaultImplsIfNeeded() { if (isInterface(descriptor) && !isLocal) { Type defaultImplsType = state.getTypeMapper().mapDefaultImpls(descriptor); ClassBuilder defaultImplsBuilder = @@ -242,7 +242,6 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { ClassContext defaultImplsContext = parentContext.intoDefaultImplsClass(descriptor, (ClassContext) context, state); new InterfaceImplBodyCodegen(myClass, defaultImplsContext, defaultImplsBuilder, state, this).generate(); } - super.generateBody(); } @Override @@ -378,7 +377,9 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { generateDelegates(delegationFieldsInfo); - generateSyntheticAccessors(); + if (!isInterface(descriptor) || kind == OwnerKind.DEFAULT_IMPLS) { + generateSyntheticAccessors(); + } generateEnumMethods(); @@ -1332,7 +1333,7 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { private void generateTraitMethods() { if (isInterface(descriptor)) return; - for (Map.Entry entry : CodegenUtil.getTraitMethods(descriptor).entrySet()) { + for (Map.Entry entry : CodegenUtil.getNonPrivateTraitMethods(descriptor).entrySet()) { FunctionDescriptor traitFun = entry.getKey(); //skip java 8 default methods if (!(traitFun instanceof JavaCallableMemberDescriptor)) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/InterfaceImplBodyCodegen.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/InterfaceImplBodyCodegen.kt index 0e7637ee236..0f6674b9cea 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/InterfaceImplBodyCodegen.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/InterfaceImplBodyCodegen.kt @@ -103,6 +103,8 @@ public class InterfaceImplBodyCodegen( } } } + + generateSyntheticAccessors() } private fun generateDelegationToSuperTraitImpl(descriptor: FunctionDescriptor, implementation: FunctionDescriptor) { diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java index cde83b16f87..d8c2d40ccb9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/MemberCodegen.java @@ -66,6 +66,7 @@ import static org.jetbrains.kotlin.codegen.AsmUtil.isPrimitive; import static org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.SYNTHESIZED; import static org.jetbrains.kotlin.resolve.BindingContext.VARIABLE; import static org.jetbrains.kotlin.resolve.DescriptorUtils.isCompanionObject; +import static org.jetbrains.kotlin.resolve.DescriptorUtils.isInterface; import static org.jetbrains.kotlin.resolve.DescriptorUtils.isStaticDeclaration; import static org.jetbrains.kotlin.resolve.jvm.AsmTypes.*; import static org.jetbrains.kotlin.resolve.jvm.diagnostics.JvmDeclarationOrigin.NO_ORIGIN; @@ -684,7 +685,7 @@ public abstract class MemberCodegen { } @NotNull + @ReadOnly public Collection> getAccessors() { return accessors == null ? Collections.>emptySet() : accessors.values(); } diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/DefaultImplsClassContext.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/DefaultImplsClassContext.kt index 707a8f35346..247d18827eb 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/context/DefaultImplsClassContext.kt +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/context/DefaultImplsClassContext.kt @@ -16,6 +16,7 @@ package org.jetbrains.kotlin.codegen.context +import org.jetbrains.kotlin.codegen.AccessorForCallableDescriptor import org.jetbrains.kotlin.codegen.OwnerKind import org.jetbrains.kotlin.codegen.state.JetTypeMapper import org.jetbrains.kotlin.descriptors.ClassDescriptor @@ -33,4 +34,11 @@ class DefaultImplsClassContext( override fun getCompanionObjectContext(): CodegenContext<*>? { return interfaceContext.companionObjectContext } + + override fun getAccessors(): Collection> { + val accessors = super.getAccessors() + val alreadyExistKeys = accessors.map ({ Pair(it.calleeDescriptor, it.superCallTarget) }) + val filtered = interfaceContext.accessors.toMap ({ Pair(it.calleeDescriptor, it.superCallTarget) }, {it}) - alreadyExistKeys + return accessors + filtered.values + } } \ No newline at end of file diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java index 587df793e5c..9dd934ca9d0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/state/JetTypeMapper.java @@ -758,7 +758,7 @@ public class JetTypeMapper { ClassDescriptor ownerForDefault = (ClassDescriptor) baseMethodDescriptor.getContainingDeclaration(); ownerForDefaultImpl = isJvmInterface(ownerForDefault) ? mapDefaultImpls(ownerForDefault) : mapClass(ownerForDefault); - if (isInterface && (superCall || descriptor.getVisibility() == Visibilities.PRIVATE)) { + if (isInterface && (superCall || descriptor.getVisibility() == Visibilities.PRIVATE || isAccessor(descriptor))) { thisClass = mapClass(currentOwner); if (declarationOwner instanceof JavaClassDescriptor) { invokeOpcode = INVOKESPECIAL; diff --git a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.NoCompile.java b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.NoCompile.java index c85d32d6619..d6b60283f92 100644 --- a/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.NoCompile.java +++ b/compiler/testData/asJava/lightClasses/nullabilityAnnotations/PrivateInTrait.NoCompile.java @@ -1,11 +1,9 @@ public interface PrivateInTrait { final class DefaultImpls { - @org.jetbrains.annotations.NotNull - static java.lang.String getNn(PrivateInTrait $this) { /* compiled code */ } + private static java.lang.String getNn(PrivateInTrait $this) { /* compiled code */ } - static void setNn(@org.jetbrains.annotations.NotNull PrivateInTrait $this, java.lang.String value) { /* compiled code */ } + private static void setNn(PrivateInTrait $this, java.lang.String value) { /* compiled code */ } - @org.jetbrains.annotations.Nullable - static java.lang.String getN(PrivateInTrait $this) { /* compiled code */ } + private static java.lang.String getN(PrivateInTrait $this) { /* compiled code */ } } } \ No newline at end of file diff --git a/compiler/testData/codegen/box/traits/noPrivateDelegation.kt b/compiler/testData/codegen/box/traits/noPrivateDelegation.kt new file mode 100644 index 00000000000..5b58f07c81b --- /dev/null +++ b/compiler/testData/codegen/box/traits/noPrivateDelegation.kt @@ -0,0 +1,16 @@ +interface Z{ + + private fun extension(): String { + return "OK" + } +} + +object Z2 : Z { + +} + +fun box() : String { + val size = Class.forName("Z2").declaredMethods.size + if (size != 0) return "fail: $size" + return "OK" +} \ No newline at end of file diff --git a/compiler/testData/codegen/box/traits/syntheticAccessor.kt b/compiler/testData/codegen/box/traits/syntheticAccessor.kt new file mode 100644 index 00000000000..32ce13d9d57 --- /dev/null +++ b/compiler/testData/codegen/box/traits/syntheticAccessor.kt @@ -0,0 +1,21 @@ +var result = "fail" + +interface B { + + private fun test() { + result = "OK" + } + + class Z { + fun ztest(b: B) { + b.test() + } + } +} + +class C : B + +fun box(): String { + B.Z().ztest(C()) + return result +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java index 7599ceaff3d..1f71348886c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java @@ -7876,6 +7876,18 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { doTest(fileName); } + @TestMetadata("noPrivateDelegation.kt") + public void testNoPrivateDelegation() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/traits/noPrivateDelegation.kt"); + doTest(fileName); + } + + @TestMetadata("syntheticAccessor.kt") + public void testSyntheticAccessor() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/traits/syntheticAccessor.kt"); + doTest(fileName); + } + @TestMetadata("traitImplDelegationWithCovariantOverride.kt") public void testTraitImplDelegationWithCovariantOverride() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/traits/traitImplDelegationWithCovariantOverride.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt index 0ba5cda844b..990dc227da0 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/ClassTranslator.kt @@ -232,7 +232,7 @@ public class ClassTranslator private constructor( } private fun generateBridgesToTraitImpl(properties: MutableList) { - for (entry in CodegenUtil.getTraitMethods(descriptor).entrySet()) { + for (entry in CodegenUtil.getNonPrivateTraitMethods(descriptor).entrySet()) { if (!areNamesEqual(entry.getKey(), entry.getValue())) { properties.add(generateDelegateCall(entry.getValue(), entry.getKey(), JsLiteral.THIS, context())) }