Merge pull request #2727 from pyos/fix-synthetic-accessors-v101

JVM_IR: add accessors for protected members in divergent hierarchies

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/SyntheticAccessorLowering.kt

@@ -60,32 +60,36 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
             return super.visitFunctionAccess(expression)
         }
 
-        fun makeFunctionAccessorSymbolWithSuper(functionSymbol: IrFunctionSymbol): IrFunctionSymbol =
-            when (functionSymbol) {
-                is IrConstructorSymbol -> functionSymbol.owner.makeConstructorAccessor().symbol
-                is IrSimpleFunctionSymbol -> functionSymbol.owner.makeSimpleFunctionAccessor(expression as IrCall).symbol
-                else -> error("Unknown subclass of IrFunctionSymbol")
-            }
-
+        val withSuper = (expression as? IrCall)?.superQualifierSymbol != null
+        val thisSymbol = (expression as? IrCall)?.dispatchReceiver?.type?.classifierOrNull as? IrClassSymbol
+        if (expression.symbol.isAccessible(withSuper, thisSymbol)) {
+            return super.visitFunctionAccess(expression)
+        }
         return super.visitExpression(
-            handleAccess(
-                expression,
-                expression.symbol,
-                functionMap,
-                ::makeFunctionAccessorSymbolWithSuper,
-                ::modifyFunctionAccessExpression,
-                (expression as? IrCall)?.superQualifierSymbol,
-                (expression as? IrCall)?.dispatchReceiver?.type?.classifierOrNull as? IrClassSymbol
-            )
+            modifyFunctionAccessExpression(expression, functionMap.getOrPut(expression.symbol) {
+                when (val symbol = expression.symbol) {
+                    is IrConstructorSymbol -> symbol.owner.makeConstructorAccessor().symbol
+                    is IrSimpleFunctionSymbol -> symbol.owner.makeSimpleFunctionAccessor(expression as IrCall).symbol
+                    else -> error("Unknown subclass of IrFunctionSymbol")
+                }
+            })
         )
     }
 
     override fun visitGetField(expression: IrGetField) = super.visitExpression(
-        handleAccess(expression, expression.symbol, getterMap, ::makeGetterAccessorSymbol, ::modifyGetterExpression)
+        if (!expression.symbol.isAccessible(false, expression.receiver?.type?.classifierOrNull as? IrClassSymbol)) {
+            modifyGetterExpression(expression, getterMap.getOrPut(expression.symbol) { makeGetterAccessorSymbol(expression.symbol) })
+        } else {
+            expression
+        }
     )
 
     override fun visitSetField(expression: IrSetField) = super.visitExpression(
-        handleAccess(expression, expression.symbol, setterMap, ::makeSetterAccessorSymbol, ::modifySetterExpression)
+        if (!expression.symbol.isAccessible(false, expression.receiver?.type?.classifierOrNull as? IrClassSymbol)) {
+            modifySetterExpression(expression, setterMap.getOrPut(expression.symbol) { makeSetterAccessorSymbol(expression.symbol) })
+        } else {
+            expression
+        }
     )
 
     override fun visitConstructor(declaration: IrConstructor): IrStatement {
@@ -139,22 +143,6 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
         }.symbol
     }
 
-    private inline fun <ExprT : IrDeclarationReference, reified FromSyT : IrSymbol, ToSyT : IrSymbol> handleAccess(
-        expression: ExprT,
-        symbol: FromSyT,
-        accumMap: MutableMap<FromSyT, ToSyT>,
-        symbolConverter: (FromSyT) -> ToSyT,
-        exprConverter: (ExprT, ToSyT) -> IrDeclarationReference,
-        superQualifierSymbol: IrClassSymbol? = null,
-        thisSymbol: IrClassSymbol? = null
-    ): IrExpression =
-        if (!symbol.isAccessible(superQualifierSymbol != null, thisSymbol)) {
-            val accessorSymbol = accumMap.getOrPut(symbol) { symbolConverter(symbol) }
-            exprConverter(expression, accessorSymbol)
-        } else {
-            expression
-        }
-
     // In case of Java `protected static`, access could be done from a public inline function in the same package,
     // or a subclass of the Java class. Both cases require an accessor, which we cannot add to the Java class.
     private fun IrDeclarationWithVisibility.accessorParent(parent: IrDeclarationParent = this.parent) =
@@ -520,17 +508,13 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
         val contextDeclarationContainer = (currentScope!!.irElement as IrDeclaration).getAccessContext(withSuper) ?: return false
 
         val samePackage = declaration.getPackageFragment()?.fqName == contextDeclarationContainer.getPackageFragment()?.fqName
+        val fromSubclassOfReceiversClass = contextDeclarationContainer is IrClass && symbolDeclarationContainer is IrClass &&
+                ((thisObjReference == null || contextDeclarationContainer.symbol.isSubtypeOfClass(thisObjReference)) &&
+                        (contextDeclarationContainer.isSubclassOf(symbolDeclarationContainer)))
         return when {
             declaration.visibility.isPrivate && symbolDeclarationContainer != contextDeclarationContainer -> false
-            declaration.visibility.isProtected && !samePackage &&
-                    !(symbolDeclarationContainer is IrClass && contextDeclarationContainer is IrClass &&
-                            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.
-            (withSuper && contextDeclarationContainer is IrClass && symbolDeclarationContainer is IrClass &&
-                    ((thisObjReference != null && !contextDeclarationContainer.symbol.isSubtypeOfClass(thisObjReference)) ||
-                            !(contextDeclarationContainer.isSubclassOf(symbolDeclarationContainer)))) -> false
+            declaration.visibility.isProtected && !samePackage && !fromSubclassOfReceiversClass -> false
+            withSuper && !fromSubclassOfReceiversClass -> false
             else -> true
         }
     }

compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/protectedAccessor.kt

@@ -0,0 +1,21 @@
+// FILE: protectedPack/A.java
+package protectedPack;
+
+public class A {
+    protected final String field;
+
+    public A(String value) {
+        field = value;
+    }
+}
+
+// FILE: B.kt
+import protectedPack.A
+
+class B(value: String) : A(value) {
+    inner class C : A(field) {
+        val result = field
+    }
+}
+
+fun box(): String = B("OK").C().result

compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxAgainstJavaCodegenTestGenerated.java

@@ -1086,6 +1086,11 @@ public class BlackBoxAgainstJavaCodegenTestGenerated extends AbstractBlackBoxAga
                 runTest("compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/overrideProtectedFunInPackage.kt");
             }
 
+            @TestMetadata("protectedAccessor.kt")
+            public void testProtectedAccessor() throws Exception {
+                runTest("compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/protectedAccessor.kt");
+            }
+
             @TestMetadata("protectedFunInPackage.kt")
             public void testProtectedFunInPackage() throws Exception {
                 runTest("compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/protectedFunInPackage.kt");

compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxAgainstJavaCodegenTestGenerated.java

@@ -1087,6 +1087,11 @@ public class IrBlackBoxAgainstJavaCodegenTestGenerated extends AbstractIrBlackBo
                 runTest("compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/overrideProtectedFunInPackage.kt");
             }
 
+            @TestMetadata("protectedAccessor.kt")
+            public void testProtectedAccessor() throws Exception {
+                runTest("compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/protectedAccessor.kt");
+            }
+
             @TestMetadata("protectedFunInPackage.kt")
             public void testProtectedFunInPackage() throws Exception {
                 runTest("compiler/testData/codegen/boxAgainstJava/visibility/protectedAndPackage/protectedFunInPackage.kt");