From 02790682140aaaccc4c279a39596230f43133308 Mon Sep 17 00:00:00 2001 From: Mads Ager Date: Fri, 30 Oct 2020 14:51:10 +0100 Subject: [PATCH] [JVM_IR] Follow old backend in bridge visibility and varargs marking. The old backend makes bridges for protected and package-private methods public. Also, for bridges for vararg methods, the vararg marker is not on the bridge. These differences seem minor but are visible via reflection, so we might as well follow the old backend. --- .../backend/jvm/codegen/FunctionCodegen.kt | 5 +- .../jvm/codegen/MethodSignatureMapper.kt | 2 +- .../backend/jvm/codegen/irCodegenUtils.kt | 55 +------------------ .../codegen/bytecodeListing/varargsBridge.kt | 8 +++ .../codegen/bytecodeListing/varargsBridge.txt | 15 +++++ .../codegen/BytecodeListingTestGenerated.java | 5 ++ .../ir/IrBytecodeListingTestGenerated.java | 5 ++ 7 files changed, 39 insertions(+), 56 deletions(-) create mode 100644 compiler/testData/codegen/bytecodeListing/varargsBridge.kt create mode 100644 compiler/testData/codegen/bytecodeListing/varargsBridge.txt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt index 4a204a4a674..ad358968031 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/FunctionCodegen.kt @@ -149,8 +149,7 @@ class FunctionCodegen( } } - val isVararg = valueParameters.lastOrNull()?.varargElementType != null - val isBridge = origin == IrDeclarationOrigin.BRIDGE || origin == IrDeclarationOrigin.BRIDGE_SPECIAL + val isVararg = valueParameters.lastOrNull()?.varargElementType != null && !isBridge() val modalityFlag = when ((this as? IrSimpleFunction)?.modality) { Modality.FINAL -> when { origin == JvmLoweredDeclarationOrigin.CLASS_STATIC_INITIALIZER -> 0 @@ -172,7 +171,7 @@ class FunctionCodegen( (if (isStatic) Opcodes.ACC_STATIC else 0) or (if (isVararg) Opcodes.ACC_VARARGS else 0) or (if (isExternal) Opcodes.ACC_NATIVE else 0) or - (if (isBridge) Opcodes.ACC_BRIDGE else 0) or + (if (isBridge()) Opcodes.ACC_BRIDGE else 0) or (if (isSynthetic) Opcodes.ACC_SYNTHETIC else 0) or (if (isStrict) Opcodes.ACC_STRICT else 0) or (if (isSynchronized) Opcodes.ACC_SYNCHRONIZED else 0) diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/MethodSignatureMapper.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/MethodSignatureMapper.kt index 216d93e55bd..31e3a2d918c 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/MethodSignatureMapper.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/MethodSignatureMapper.kt @@ -362,7 +362,7 @@ class MethodSignatureMapper(private val context: JvmBackendContext) { private fun mapOverriddenSpecialBuiltinIfNeeded(caller: IrFunction, callee: IrFunction, superCall: Boolean): JvmMethodSignature? { // Do not remap special builtin methods when called from a bridge. The bridges are there to provide the // remapped name or signature and forward to the actually declared method. - if (caller.origin == IrDeclarationOrigin.BRIDGE || caller.origin == IrDeclarationOrigin.BRIDGE_SPECIAL) return null + if (caller.isBridge()) return null // Do not remap calls to static replacements of inline class methods, since they have completely different signatures. if (callee.isStaticInlineClassReplacement) return null val overriddenSpecialBuiltinFunction = diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt index b1762175b4a..c2d31234600 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/irCodegenUtils.kt @@ -168,21 +168,16 @@ fun IrDeclarationWithVisibility.getVisibilityAccessFlag(kind: OwnerKind? = null) } private fun IrDeclarationWithVisibility.specialCaseVisibility(kind: OwnerKind?): Int? { -// if (JvmCodegenUtil.isNonIntrinsicPrivateCompanionObjectInInterface(memberDescriptor)) { -// return ACC_PUBLIC -// } if (this is IrClass && DescriptorVisibilities.isPrivate(visibility) && isCompanion && hasInterfaceParent()) { // TODO: non-intrinsic return Opcodes.ACC_PUBLIC } -// if (memberDescriptor is FunctionDescriptor && isInlineClassWrapperConstructor(memberDescriptor, kind)) if (this is IrConstructor && parentAsClass.isInline && kind === OwnerKind.IMPLEMENTATION) { return Opcodes.ACC_PRIVATE } -// if (memberDescriptor.isEffectivelyInlineOnly()) { - if (this is IrFunction && isReifiable()) { + if (this is IrFunction && (isReifiable() || isBridge())) { return Opcodes.ACC_PUBLIC } @@ -190,58 +185,20 @@ private fun IrDeclarationWithVisibility.specialCaseVisibility(kind: OwnerKind?): return Opcodes.ACC_PRIVATE } -// if (memberVisibility === Visibilities.LOCAL && memberDescriptor is CallableMemberDescriptor) { if (visibility === DescriptorVisibilities.LOCAL && this is IrFunction) { return Opcodes.ACC_PUBLIC } -// if (isEnumEntry(memberDescriptor)) { if (this is IrClass && this.kind === ClassKind.ENUM_ENTRY) { return AsmUtil.NO_FLAG_PACKAGE_PRIVATE } -// These ones should be public anyway after ToArrayLowering. -// if (memberDescriptor.isToArrayFromCollection()) { -// return ACC_PUBLIC -// } - -// if (memberDescriptor is ConstructorDescriptor && isAnonymousObject(memberDescriptor.containingDeclaration)) { -// return getVisibilityAccessFlagForAnonymous(memberDescriptor.containingDeclaration as ClassDescriptor) -// } -// if (this is IrConstructor && parentAsClass.isAnonymousObject) { -// return parentAsClass.getVisibilityAccessFlagForAnonymous() -// } - -// TODO: when is this applicable? -// if (memberDescriptor is SyntheticJavaPropertyDescriptor) { -// return getVisibilityAccessFlag((memberDescriptor as SyntheticJavaPropertyDescriptor).getMethod) -// } - - -// if (memberDescriptor is PropertyAccessorDescriptor) { -// val property = memberDescriptor.correspondingProperty -// if (property is SyntheticJavaPropertyDescriptor) { -// val method = (if (memberDescriptor === property.getGetter()) -// (property as SyntheticJavaPropertyDescriptor).getMethod -// else -// (property as SyntheticJavaPropertyDescriptor).setMethod) -// ?: error("No get/set method in SyntheticJavaPropertyDescriptor: $property") -// return getVisibilityAccessFlag(method) -// } -// } if (this is IrField && correspondingPropertySymbol?.owner?.isExternal == true) { val method = correspondingPropertySymbol?.owner?.getter ?: correspondingPropertySymbol?.owner?.setter ?: error("No get/set method in SyntheticJavaPropertyDescriptor: ${ir2string(correspondingPropertySymbol?.owner)}") return method.getVisibilityAccessFlag() } -// if (memberDescriptor is CallableDescriptor && memberVisibility === Visibilities.PROTECTED) { -// for (overridden in DescriptorUtils.getAllOverriddenDescriptors(memberDescriptor as CallableDescriptor)) { -// if (isJvmInterface(overridden.containingDeclaration)) { -// return ACC_PUBLIC -// } -// } -// } if (this is IrSimpleFunction && visibility === DescriptorVisibilities.PROTECTED && allOverridden().any { it.parentAsClass.isJvmInterface } ) { @@ -256,14 +213,6 @@ private fun IrDeclarationWithVisibility.specialCaseVisibility(kind: OwnerKind?): return AsmUtil.NO_FLAG_PACKAGE_PRIVATE } -// Should be taken care of in IR -// if (memberDescriptor is AccessorForCompanionObjectInstanceFieldDescriptor) { -// return NO_FLAG_PACKAGE_PRIVATE -// } - -// return if (memberDescriptor is ConstructorDescriptor && isEnumEntry(containingDeclaration)) { -// NO_FLAG_PACKAGE_PRIVATE -// } else null if (this is IrConstructor && parentAsClass.kind === ClassKind.ENUM_ENTRY) { return AsmUtil.NO_FLAG_PACKAGE_PRIVATE } @@ -303,6 +252,8 @@ fun IrFunction.isInlineOnly() = fun IrFunction.isReifiable() = typeParameters.any { it.isReified } +internal fun IrFunction.isBridge() = origin == IrDeclarationOrigin.BRIDGE || origin == IrDeclarationOrigin.BRIDGE_SPECIAL + // Borrowed with modifications from ImplementationBodyCodegen.java private val KOTLIN_MARKER_INTERFACES: Map = run { diff --git a/compiler/testData/codegen/bytecodeListing/varargsBridge.kt b/compiler/testData/codegen/bytecodeListing/varargsBridge.kt new file mode 100644 index 00000000000..acaf8fb1a19 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/varargsBridge.kt @@ -0,0 +1,8 @@ +abstract class A { + protected abstract fun doIt(vararg args: T): String + fun test() = doIt() +} + +class B : A() { + override fun doIt(vararg args: Void): String = "OK" +} diff --git a/compiler/testData/codegen/bytecodeListing/varargsBridge.txt b/compiler/testData/codegen/bytecodeListing/varargsBridge.txt new file mode 100644 index 00000000000..00944afe423 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/varargsBridge.txt @@ -0,0 +1,15 @@ +@kotlin.Metadata +public abstract class A { + // source: 'varargsBridge.kt' + public method (): void + protected varargs abstract @org.jetbrains.annotations.NotNull method doIt(@org.jetbrains.annotations.NotNull p0: java.lang.Object[]): java.lang.String + public final @org.jetbrains.annotations.NotNull method test(): java.lang.String +} + +@kotlin.Metadata +public final class B { + // source: 'varargsBridge.kt' + public method (): void + protected varargs @org.jetbrains.annotations.NotNull method doIt(@org.jetbrains.annotations.NotNull p0: java.lang.Void[]): java.lang.String + public synthetic bridge method doIt(p0: java.lang.Object[]): java.lang.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java index 9853ee6037d..b69fb42e396 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java @@ -179,6 +179,11 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest { runTest("compiler/testData/codegen/bytecodeListing/samAdapterAndInlinedOne.kt"); } + @TestMetadata("varargsBridge.kt") + public void testVarargsBridge() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/varargsBridge.kt"); + } + @TestMetadata("compiler/testData/codegen/bytecodeListing/annotations") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java index aafa9cca057..78b2218591c 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBytecodeListingTestGenerated.java @@ -179,6 +179,11 @@ public class IrBytecodeListingTestGenerated extends AbstractIrBytecodeListingTes runTest("compiler/testData/codegen/bytecodeListing/samAdapterAndInlinedOne.kt"); } + @TestMetadata("varargsBridge.kt") + public void testVarargsBridge() throws Exception { + runTest("compiler/testData/codegen/bytecodeListing/varargsBridge.kt"); + } + @TestMetadata("compiler/testData/codegen/bytecodeListing/annotations") @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class)