From 4cd4ea126bed6d51c61e8547c91d0e338e9c8d05 Mon Sep 17 00:00:00 2001 From: pyos Date: Wed, 22 May 2019 16:37:29 +0200 Subject: [PATCH] JVM_IR: generate more accessors for use in inline functions --- .../jvm/lower/SyntheticAccessorLowering.kt | 75 +++++++------------ .../properties/protectedJavaFieldInInline.kt | 1 - .../properties/localDelegated/inlineFun.kt | 1 - ...inlineCallsStaticMethodFromOtherPackage.kt | 25 +++++++ .../box/statics/protectedStaticAndInline.kt | 1 - .../boxInline/delegatedProperty/local.kt | 1 - .../codegen/boxInline/private/kt6453.kt | 1 - .../codegen/boxInline/private/kt8094.kt | 1 - .../codegen/boxInline/private/kt8095.kt | 1 - .../boxInline/private/nestedInPrivateClass.kt | 1 - .../codegen/boxInline/private/privateClass.kt | 1 - .../boxInline/private/privateInline.kt | 1 - .../packagePrivateMembers.kt | 1 - .../syntheticAccessors/protectedMembers.kt | 1 - .../protectedMembersFromSuper.kt | 1 - .../boxInline/syntheticAccessors/superCall.kt | 1 - .../syntheticAccessors/superProperty.kt | 1 - .../codegen/BlackBoxCodegenTestGenerated.java | 5 ++ .../LightAnalysisModeTestGenerated.java | 5 ++ .../ir/IrBlackBoxCodegenTestGenerated.java | 5 ++ 20 files changed, 68 insertions(+), 62 deletions(-) create mode 100644 compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt index 0230384866f..1a4f3c1e195 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt @@ -7,8 +7,6 @@ package org.jetbrains.kotlin.backend.jvm.lower import org.jetbrains.kotlin.backend.common.FileLoweringPass import org.jetbrains.kotlin.backend.common.IrElementTransformerVoidWithContext -import org.jetbrains.kotlin.backend.common.IrElementVisitorVoidWithContext -import org.jetbrains.kotlin.backend.common.ScopeWithIr import org.jetbrains.kotlin.backend.common.ir.copyTypeParametersFrom import org.jetbrains.kotlin.backend.common.ir.copyValueParametersToStatic import org.jetbrains.kotlin.backend.common.ir.passTypeArgumentsFrom @@ -20,6 +18,7 @@ import org.jetbrains.kotlin.backend.jvm.intrinsics.receiverAndArgs import org.jetbrains.kotlin.codegen.OwnerKind import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.descriptors.Visibilities +import org.jetbrains.kotlin.descriptors.Visibility import org.jetbrains.kotlin.ir.IrElement import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET import org.jetbrains.kotlin.ir.builders.declarations.addValueParameter @@ -30,12 +29,10 @@ import org.jetbrains.kotlin.ir.expressions.* import org.jetbrains.kotlin.ir.expressions.impl.* import org.jetbrains.kotlin.ir.symbols.* import org.jetbrains.kotlin.ir.types.classifierOrNull -import org.jetbrains.kotlin.ir.types.impl.IrSimpleTypeImpl import org.jetbrains.kotlin.ir.types.isSubtypeOfClass import org.jetbrains.kotlin.ir.util.* -import org.jetbrains.kotlin.ir.visitors.acceptChildrenVoid -import org.jetbrains.kotlin.ir.visitors.acceptVoid import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid +import org.jetbrains.kotlin.load.java.JavaVisibilities import org.jetbrains.kotlin.load.java.JvmAbi import org.jetbrains.kotlin.name.Name @@ -48,10 +45,8 @@ internal val syntheticAccessorPhase = makeIrFilePhase( private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElementTransformerVoidWithContext(), FileLoweringPass { private val pendingTransformations = mutableListOf>() - private val inlinedLambdasCollector = InlinedLambdasCollector() override fun lower(irFile: IrFile) { - irFile.acceptVoid(inlinedLambdasCollector) irFile.transformChildrenVoid(this) pendingTransformations.forEach { it() } } @@ -110,6 +105,14 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem else -> error("Unknown subclass of IrFunctionSymbol") } + // In case of Java `protected static`, access could be done from a public inline function in the same package, + // requiring an accessor, which we cannot add to the Java class. + private fun IrDeclarationWithVisibility.accessorParent() = + if (visibility == JavaVisibilities.PROTECTED_STATIC_VISIBILITY) + currentClass!!.irElement as IrClass + else + parent + private fun IrConstructor.makeConstructorAccessor(): IrConstructor { val source = this @@ -161,7 +164,7 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem // TODO: Handle the exception after targeting Java 8 or newer, where JVM default methods are available. context.declarationFactory.getDefaultImplsClass(source.parent as IrClass) } else { - source.parent + source.accessorParent() } pendingTransformations.add { (accessor.parent as IrDeclarationContainer).declarations.add(accessor) } @@ -202,7 +205,7 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem modality = Modality.FINAL returnType = fieldSymbol.owner.type }.also { accessor -> - accessor.parent = fieldSymbol.owner.parent + accessor.parent = fieldSymbol.owner.accessorParent() pendingTransformations.add { (accessor.parent as IrDeclarationContainer).declarations.add(accessor) } if (!fieldSymbol.owner.isStatic) { @@ -238,7 +241,7 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem modality = Modality.FINAL returnType = context.irBuiltIns.unitType }.also { accessor -> - accessor.parent = fieldSymbol.owner.parent + accessor.parent = fieldSymbol.owner.accessorParent() pendingTransformations.add { (accessor.parent as IrDeclarationContainer).declarations.add(accessor) } if (!fieldSymbol.owner.isStatic) { @@ -406,6 +409,14 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem return Name.identifier("access\$prop\$$setterName") } + private val Visibility.isPrivate + get() = Visibilities.isPrivate(this) + + private val Visibility.isProtected + get() = this == Visibilities.PROTECTED || + this == JavaVisibilities.PROTECTED_AND_PACKAGE || + this == JavaVisibilities.PROTECTED_STATIC_VISIBILITY + private fun IrSymbol.isAccessible(withSuper: Boolean, thisObjReference: IrClassSymbol?): Boolean { /// We assume that IR code that reaches us has been checked for correctness at the frontend. /// This function needs to single out those cases where Java accessibility rules differ from Kotlin's. @@ -418,24 +429,25 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem // There is never a problem with visibility of inline functions, as those don't end up as Java entities if (declaration is IrFunction && declaration.isInline) return true - // The only two visibilities where Kotlin rules differ from JVM rules. - if (!withSuper && !Visibilities.isPrivate(declaration.visibility) && declaration.visibility != Visibilities.PROTECTED) return true + // `internal` maps to public and requires no accessor. + if (!withSuper && !declaration.visibility.isPrivate && !declaration.visibility.isProtected) return true // If local variables are accessible by Kotlin rules, they also are by Java rules. val symbolDeclarationContainer = (declaration.parent as? IrDeclarationContainer) as? IrElement ?: return true // Within inline functions, we have to assume the worst. - if (inlinedLambdasCollector.isInlineNonpublicContext(allScopes)) + val function = currentFunction?.irElement as IrFunction? + if (function?.isInline == true && !function.visibility.isPrivate && (withSuper || !function.visibility.isProtected)) return false val contextDeclarationContainer = allScopes.lastOrNull { it.irElement is IrDeclarationContainer }?.irElement val samePackage = declaration.getPackageFragment()?.fqName == contextDeclarationContainer?.getPackageFragment()?.fqName return when { - Visibilities.isPrivate(declaration.visibility) && symbolDeclarationContainer != contextDeclarationContainer -> false - (declaration.visibility == Visibilities.PROTECTED && !samePackage && + declaration.visibility.isPrivate && symbolDeclarationContainer != contextDeclarationContainer -> false + declaration.visibility.isProtected && !samePackage && !(symbolDeclarationContainer is IrClass && contextDeclarationContainer is IrClass && - contextDeclarationContainer.isSubclassOf(symbolDeclarationContainer))) -> false + contextDeclarationContainer.isSubclassOf(symbolDeclarationContainer)) -> false // Invoking with super qualifier is implemented by invokespecial, which requires // 1. `this` to be assign compatible with current class. // 2. the method is a member of a superclass of current class. @@ -446,34 +458,3 @@ private class SyntheticAccessorLowering(val context: JvmBackendContext) : IrElem } } } - -private class InlinedLambdasCollector : IrElementVisitorVoidWithContext() { - private val inlinedLambdasInNonPublicContexts = mutableSetOf() - - fun isInlineNonpublicContext(scopeStack: List): Boolean { - val currentFunction = scopeStack.map { it.irElement }.lastOrNull { it is IrFunction } as? IrFunction ?: return false - return (currentFunction.isInline && currentFunction.visibility in setOf(Visibilities.PRIVATE, Visibilities.PROTECTED)) - || currentFunction in inlinedLambdasInNonPublicContexts - } - - override fun visitElement(element: IrElement) { - element.acceptChildrenVoid(this) - } - - override fun visitFunctionAccess(expression: IrFunctionAccessExpression) { - val callTarget = expression.symbol.owner - if (callTarget.isInline && isInlineNonpublicContext(allScopes)) { - for (i in 0 until expression.valueArgumentsCount) { - val argument = expression.getValueArgument(i)?.removeBlocks() - if (argument is IrFunctionReference && - argument.symbol.owner.origin == IrDeclarationOrigin.LOCAL_FUNCTION_FOR_LAMBDA && - !callTarget.valueParameters[i].isNoinline - ) { - inlinedLambdasInNonPublicContexts.add(argument.symbol.owner) - } - } - } - super.visitFunctionAccess(expression) - } -} - diff --git a/compiler/testData/codegen/box/properties/protectedJavaFieldInInline.kt b/compiler/testData/codegen/box/properties/protectedJavaFieldInInline.kt index 08bb29ad05e..869c8440d9c 100644 --- a/compiler/testData/codegen/box/properties/protectedJavaFieldInInline.kt +++ b/compiler/testData/codegen/box/properties/protectedJavaFieldInInline.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // FILE: JavaClass.java diff --git a/compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt b/compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt index bc77c8f95d9..74716048087 100644 --- a/compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt +++ b/compiler/testData/codegen/box/reflection/properties/localDelegated/inlineFun.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // WITH_REFLECT diff --git a/compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt b/compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt new file mode 100644 index 00000000000..69db230eb43 --- /dev/null +++ b/compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt @@ -0,0 +1,25 @@ +// TARGET_BACKEND: JVM +// The non-IR backend attempts to call a non-existent accessor in class Test. +// IGNORE_BACKEND: JVM + +// FILE: Test.java + +public class Test { + protected static String testStatic() { + return "OK"; + } +} + +// FILE: test.kt + +class Test2 { + inline fun test() = Test.testStatic() +} + +// FILE: test2.kt + +package anotherPackage + +import Test2 + +fun box() = Test2().test() diff --git a/compiler/testData/codegen/box/statics/protectedStaticAndInline.kt b/compiler/testData/codegen/box/statics/protectedStaticAndInline.kt index b7f94ebc0db..efa04f27fb5 100644 --- a/compiler/testData/codegen/box/statics/protectedStaticAndInline.kt +++ b/compiler/testData/codegen/box/statics/protectedStaticAndInline.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // TARGET_BACKEND: JVM // FILE: First.java diff --git a/compiler/testData/codegen/boxInline/delegatedProperty/local.kt b/compiler/testData/codegen/boxInline/delegatedProperty/local.kt index 8b1801e5c55..36fc9fc367e 100644 --- a/compiler/testData/codegen/boxInline/delegatedProperty/local.kt +++ b/compiler/testData/codegen/boxInline/delegatedProperty/local.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/private/kt6453.kt b/compiler/testData/codegen/boxInline/private/kt6453.kt index a4192e170d4..1714c3ebcbf 100644 --- a/compiler/testData/codegen/boxInline/private/kt6453.kt +++ b/compiler/testData/codegen/boxInline/private/kt6453.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/private/kt8094.kt b/compiler/testData/codegen/boxInline/private/kt8094.kt index 9abcba98bb3..ef22ca47d34 100644 --- a/compiler/testData/codegen/boxInline/private/kt8094.kt +++ b/compiler/testData/codegen/boxInline/private/kt8094.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/private/kt8095.kt b/compiler/testData/codegen/boxInline/private/kt8095.kt index 7e4200072c6..3cd6ed10474 100644 --- a/compiler/testData/codegen/boxInline/private/kt8095.kt +++ b/compiler/testData/codegen/boxInline/private/kt8095.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/private/nestedInPrivateClass.kt b/compiler/testData/codegen/boxInline/private/nestedInPrivateClass.kt index 89a294789ac..13bc819cd1a 100644 --- a/compiler/testData/codegen/boxInline/private/nestedInPrivateClass.kt +++ b/compiler/testData/codegen/boxInline/private/nestedInPrivateClass.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/private/privateClass.kt b/compiler/testData/codegen/boxInline/private/privateClass.kt index 36ea53f7352..4ffa7890dda 100644 --- a/compiler/testData/codegen/boxInline/private/privateClass.kt +++ b/compiler/testData/codegen/boxInline/private/privateClass.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/private/privateInline.kt b/compiler/testData/codegen/boxInline/private/privateInline.kt index fe05fa7f85a..13c632f2e17 100644 --- a/compiler/testData/codegen/boxInline/private/privateInline.kt +++ b/compiler/testData/codegen/boxInline/private/privateInline.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/syntheticAccessors/packagePrivateMembers.kt b/compiler/testData/codegen/boxInline/syntheticAccessors/packagePrivateMembers.kt index 5a90fa3ad68..30bef9d6bc8 100644 --- a/compiler/testData/codegen/boxInline/syntheticAccessors/packagePrivateMembers.kt +++ b/compiler/testData/codegen/boxInline/syntheticAccessors/packagePrivateMembers.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembers.kt b/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembers.kt index 179dc1067bd..21bcb3291ee 100644 --- a/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembers.kt +++ b/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembers.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembersFromSuper.kt b/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembersFromSuper.kt index 106bbea010b..93daa050549 100644 --- a/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembersFromSuper.kt +++ b/compiler/testData/codegen/boxInline/syntheticAccessors/protectedMembersFromSuper.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt b/compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt index 7ee82c75795..98cf3d927ed 100644 --- a/compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt +++ b/compiler/testData/codegen/boxInline/syntheticAccessors/superCall.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt b/compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt index e3dd1a22d45..e5f900da71e 100644 --- a/compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt +++ b/compiler/testData/codegen/boxInline/syntheticAccessors/superProperty.kt @@ -1,4 +1,3 @@ -// IGNORE_BACKEND: JVM_IR // FILE: 1.kt package test diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java index 93ef920df8b..1a061e31d0d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java @@ -23813,6 +23813,11 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/statics/inlineCallsStaticMethod.kt"); } + @TestMetadata("inlineCallsStaticMethodFromOtherPackage.kt") + public void testInlineCallsStaticMethodFromOtherPackage() throws Exception { + runTest("compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt"); + } + @TestMetadata("kt8089.kt") public void testKt8089() throws Exception { runTest("compiler/testData/codegen/box/statics/kt8089.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index e7b2a981f85..fe1cf5216a3 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -23755,6 +23755,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes @TestDataPath("$PROJECT_ROOT") @RunWith(JUnit3RunnerWithInners.class) public static class Statics extends AbstractLightAnalysisModeTest { + @TestMetadata("inlineCallsStaticMethodFromOtherPackage.kt") + public void ignoreInlineCallsStaticMethodFromOtherPackage() throws Exception { + runTest("compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt"); + } + private void runTest(String testDataFilePath) throws Exception { KotlinTestUtils.runTest(this::doTest, TargetBackend.JVM, testDataFilePath); } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java index cd4c1fe53b1..ada29893928 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java @@ -23833,6 +23833,11 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/statics/inlineCallsStaticMethod.kt"); } + @TestMetadata("inlineCallsStaticMethodFromOtherPackage.kt") + public void testInlineCallsStaticMethodFromOtherPackage() throws Exception { + runTest("compiler/testData/codegen/box/statics/inlineCallsStaticMethodFromOtherPackage.kt"); + } + @TestMetadata("kt8089.kt") public void testKt8089() throws Exception { runTest("compiler/testData/codegen/box/statics/kt8089.kt");