From 5059513106e2c82c37ed8dd5f0a5fe6cff927726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Sch=C3=A4fer?= Date: Fri, 6 Aug 2021 10:27:13 +0200 Subject: [PATCH] jvm-abi-gen: Strip inline functions in private classes Inline functions in private classes can only be accessed from Java and we do not need to preserve their bodies for the abi. --- .../kotlin/resolve/inline/InlineUtil.java | 8 ++++-- .../kotlin/backend/jvm/ir/IrUtils.kt | 26 +++++++++++++++---- .../jvm/abi/JvmAbiClassBuilderInterceptor.kt | 17 ++++++++---- .../jvm/abi/CompareJvmAbiTestGenerated.java | 10 +++++++ .../jvm/abi/IrCompareJvmAbiTestGenerated.java | 10 +++++++ .../abi/LegacyCompareJvmAbiTestGenerated.java | 10 +++++++ .../base/anonymousObject.kt | 9 +++++++ .../inlineFunInPrivateClass/directives.txt | 1 + .../sameAbi/anonymousObject.kt | 9 +++++++ .../base/anonymousObject.kt | 11 ++++++++ .../directives.txt | 1 + .../sameAbi/anonymousObject.kt | 11 ++++++++ 12 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/base/anonymousObject.kt create mode 100644 plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/directives.txt create mode 100644 plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/sameAbi/anonymousObject.kt create mode 100644 plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/base/anonymousObject.kt create mode 100644 plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/directives.txt create mode 100644 plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/sameAbi/anonymousObject.kt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/inline/InlineUtil.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/inline/InlineUtil.java index 1eab134872c..a9111ff54df 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/inline/InlineUtil.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/inline/InlineUtil.java @@ -25,6 +25,7 @@ import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.psi.*; import org.jetbrains.kotlin.resolve.BindingContext; import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; +import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext; import org.jetbrains.kotlin.resolve.calls.model.ArgumentMapping; @@ -78,8 +79,11 @@ public class InlineUtil { if (descriptor == null) return false; if (isInline(descriptor) && descriptor instanceof DeclarationDescriptorWithVisibility) { DescriptorVisibility visibility = ((DeclarationDescriptorWithVisibility) descriptor).getVisibility(); - if (!DescriptorVisibilities.isPrivate(visibility)) - return true; + if (!DescriptorVisibilities.isPrivate(visibility)) { + ClassDescriptor containingClass = DescriptorUtils.getContainingClass(descriptor); + if (containingClass == null || !DescriptorVisibilities.isPrivate(containingClass.getVisibility())) + return true; + } } return isInPublicInlineScope(descriptor.getContainingDeclaration()); } diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrUtils.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrUtils.kt index fbe6a0f2177..6a740d01f65 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrUtils.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/IrUtils.kt @@ -6,14 +6,20 @@ package org.jetbrains.kotlin.backend.jvm.ir import org.jetbrains.kotlin.backend.common.ir.ir2string -import org.jetbrains.kotlin.backend.jvm.* +import org.jetbrains.kotlin.backend.jvm.CachedFieldsForObjectInstances +import org.jetbrains.kotlin.backend.jvm.JvmBackendContext +import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin +import org.jetbrains.kotlin.backend.jvm.JvmSymbols import org.jetbrains.kotlin.backend.jvm.codegen.isInlineOnly import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface import org.jetbrains.kotlin.backend.jvm.codegen.representativeUpperBound import org.jetbrains.kotlin.backend.jvm.lower.inlineclasses.unboxInlineClass import org.jetbrains.kotlin.codegen.inline.coroutines.FOR_INLINE_SUFFIX import org.jetbrains.kotlin.config.JvmDefaultMode -import org.jetbrains.kotlin.descriptors.* +import org.jetbrains.kotlin.descriptors.ClassKind +import org.jetbrains.kotlin.descriptors.DescriptorVisibilities +import org.jetbrains.kotlin.descriptors.DescriptorVisibility +import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.descriptors.deserialization.PLATFORM_DEPENDENT_ANNOTATION_FQ_NAME import org.jetbrains.kotlin.ir.IrBuiltIns import org.jetbrains.kotlin.ir.IrElement @@ -426,15 +432,25 @@ private val IrDeclaration.original: IrDeclaration // Declarations in the scope of an externally visible inline function are implicitly part of the // public ABI of a Kotlin module. This function returns the visibility of a containing inline function // (determined *before* lowering), or null if the given declaration is not in the scope of an inline function. +// +// Currently, we mark all declarations in the scope of a public inline function as public, even if they are +// contained in a nested private inline function. This is an over approximation, since private declarations +// inside of a public inline function can still escape if they are used without being regenerated. +// See `plugins/jvm-abi-gen/testData/compile/inlineNoRegeneration` for an example. val IrDeclaration.inlineScopeVisibility: DescriptorVisibility? get() { var owner: IrDeclaration? = original var result: DescriptorVisibility? = null while (owner != null) { if (owner is IrFunction && owner.isInline) { - if (!DescriptorVisibilities.isPrivate(owner.visibility)) - return owner.visibility - result = owner.visibility + result = if (!DescriptorVisibilities.isPrivate(owner.visibility)) { + if (owner.parentClassOrNull?.visibility?.let(DescriptorVisibilities::isPrivate) == true) + DescriptorVisibilities.PRIVATE + else + return owner.visibility + } else { + owner.visibility + } } owner = owner.parent.safeAs()?.original } diff --git a/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiClassBuilderInterceptor.kt b/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiClassBuilderInterceptor.kt index 176168c7c8e..2f079b0276b 100644 --- a/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiClassBuilderInterceptor.kt +++ b/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiClassBuilderInterceptor.kt @@ -12,6 +12,7 @@ import org.jetbrains.kotlin.codegen.DelegatingClassBuilder import org.jetbrains.kotlin.codegen.DelegatingClassBuilderFactory import org.jetbrains.kotlin.codegen.extensions.ClassBuilderInterceptorExtension import org.jetbrains.kotlin.codegen.inline.coroutines.FOR_INLINE_SUFFIX +import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.DescriptorVisibilities import org.jetbrains.kotlin.descriptors.FunctionDescriptor import org.jetbrains.kotlin.descriptors.MemberDescriptor @@ -65,11 +66,17 @@ class JvmAbiClassBuilderInterceptor : ClassBuilderInterceptorExtension { override fun interceptClassBuilderFactory(interceptedFactory: ClassBuilderFactory, bindingContext: BindingContext, diagnostics: DiagnosticSink): ClassBuilderFactory = object : DelegatingClassBuilderFactory(interceptedFactory) { - override fun newClassBuilder(origin: JvmDeclarationOrigin): DelegatingClassBuilder = - AbiInfoClassBuilder(interceptedFactory.newClassBuilder(origin)) + override fun newClassBuilder(origin: JvmDeclarationOrigin): DelegatingClassBuilder { + val descriptor = origin.descriptor as? ClassDescriptor + val isPrivate = descriptor?.visibility?.let(DescriptorVisibilities::isPrivate) ?: false + return AbiInfoClassBuilder(interceptedFactory.newClassBuilder(origin), isPrivate) + } } - private inner class AbiInfoClassBuilder(private val delegate: ClassBuilder) : DelegatingClassBuilder() { + private inner class AbiInfoClassBuilder( + private val delegate: ClassBuilder, + private val isPrivateClass: Boolean + ) : DelegatingClassBuilder() { lateinit var internalName: String var localOrAnonymousClass = false var publicAbi = false @@ -104,7 +111,7 @@ class JvmAbiClassBuilderInterceptor : ClassBuilderInterceptorExtension { // metadata to indicate this (the inliner simply first checks for a method such as `f$$forInline` // and then checks for `f` if this method doesn't exist) so we have to remember to strip the // original methods if there was a $$forInline version. - if (name.endsWith(FOR_INLINE_SUFFIX)) { + if (name.endsWith(FOR_INLINE_SUFFIX) && !isPrivateClass) { // Note that origin.descriptor is null on the JVM BE in this case. methodInfos[Method(name, desc)] = AbiMethodInfo.KEEP maskedMethods += Method(name.removeSuffix(FOR_INLINE_SUFFIX), desc) @@ -119,7 +126,7 @@ class JvmAbiClassBuilderInterceptor : ClassBuilderInterceptorExtension { } // Copy inline functions verbatim - if (origin.descriptor?.safeAs()?.isInline == true) { + if (origin.descriptor?.safeAs()?.isInline == true && !isPrivateClass) { methodInfos[Method(name, desc)] = AbiMethodInfo.KEEP } else { methodInfos[Method(name, desc)] = AbiMethodInfo.STRIP diff --git a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java index de4cb9e4cb8..cb2e6440492 100644 --- a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java +++ b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java @@ -85,6 +85,16 @@ public class CompareJvmAbiTestGenerated extends AbstractCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/functionBody/"); } + @TestMetadata("inlineFunInPrivateClass") + public void testInlineFunInPrivateClass() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/"); + } + + @TestMetadata("inlineFunInPrivateNestedClass") + public void testInlineFunInPrivateNestedClass() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/"); + } + @TestMetadata("inlineFunctionBody") public void testInlineFunctionBody() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/inlineFunctionBody/"); diff --git a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java index be790591934..91bef552f82 100644 --- a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java +++ b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java @@ -85,6 +85,16 @@ public class IrCompareJvmAbiTestGenerated extends AbstractIrCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/functionBody/"); } + @TestMetadata("inlineFunInPrivateClass") + public void testInlineFunInPrivateClass() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/"); + } + + @TestMetadata("inlineFunInPrivateNestedClass") + public void testInlineFunInPrivateNestedClass() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/"); + } + @TestMetadata("inlineFunctionBody") public void testInlineFunctionBody() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/inlineFunctionBody/"); diff --git a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java index 305216a0c75..e6e5ff145f3 100644 --- a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java +++ b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java @@ -85,6 +85,16 @@ public class LegacyCompareJvmAbiTestGenerated extends AbstractLegacyCompareJvmAb runTest("plugins/jvm-abi-gen/testData/compare/functionBody/"); } + @TestMetadata("inlineFunInPrivateClass") + public void testInlineFunInPrivateClass() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/"); + } + + @TestMetadata("inlineFunInPrivateNestedClass") + public void testInlineFunInPrivateNestedClass() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/"); + } + @TestMetadata("inlineFunctionBody") public void testInlineFunctionBody() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/inlineFunctionBody/"); diff --git a/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/base/anonymousObject.kt b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/base/anonymousObject.kt new file mode 100644 index 00000000000..fdd7de502f7 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/base/anonymousObject.kt @@ -0,0 +1,9 @@ +private class A { + inline fun test(crossinline s: () -> Unit) { + object { + fun run() { + s() + } + }.run() + } +} diff --git a/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/directives.txt b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/directives.txt new file mode 100644 index 00000000000..3f7eb8b3500 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/directives.txt @@ -0,0 +1 @@ +// IGNORE_BACKEND_LEGACY: JVM \ No newline at end of file diff --git a/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/sameAbi/anonymousObject.kt b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/sameAbi/anonymousObject.kt new file mode 100644 index 00000000000..7fb3b194a46 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateClass/sameAbi/anonymousObject.kt @@ -0,0 +1,9 @@ +private class A { + inline fun test(crossinline s: () -> Unit) { + object { + fun run() { + //s() + } + }.run() + } +} diff --git a/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/base/anonymousObject.kt b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/base/anonymousObject.kt new file mode 100644 index 00000000000..6493cee8b4c --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/base/anonymousObject.kt @@ -0,0 +1,11 @@ +class A { + private class B { + inline fun test(crossinline s: () -> Unit) { + object { + fun run() { + s() + } + }.run() + } + } +} diff --git a/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/directives.txt b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/directives.txt new file mode 100644 index 00000000000..3f7eb8b3500 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/directives.txt @@ -0,0 +1 @@ +// IGNORE_BACKEND_LEGACY: JVM \ No newline at end of file diff --git a/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/sameAbi/anonymousObject.kt b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/sameAbi/anonymousObject.kt new file mode 100644 index 00000000000..1b7d4766100 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/inlineFunInPrivateNestedClass/sameAbi/anonymousObject.kt @@ -0,0 +1,11 @@ +class A { + private class B { + inline fun test(crossinline s: () -> Unit) { + object { + fun run() { + //s() + } + }.run() + } + } +}