KT-14258 Optimize accesses to properties defined into companion

- Use direct access to property defined into companion object when
it is possible rather than always use an accessor to access the
property.
- Use direct access will speedup runtime performance.
- Avoid to generate useless accessors for companion properties.

Fix of https://youtrack.jetbrains.com/issue/KT-14258

compiler/backend/src/org/jetbrains/kotlin/codegen/ExpressionCodegen.java

@@ -1950,6 +1950,10 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
         }
     }
 
+    private static boolean isDefaultAccessor(@Nullable PropertyAccessorDescriptor accessor) {
+        return accessor == null || accessor.isDefault();
+    }
+
     public StackValue.Property intermediateValueForProperty(
             @NotNull PropertyDescriptor propertyDescriptor,
             boolean forceField,
@@ -1976,7 +1980,9 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
             fieldAccessorKind = FieldAccessorKind.LATEINIT_INTRINSIC;
         }
         else if (isBackingFieldInClassCompanion &&
-            (forceField || propertyDescriptor.isConst() && Visibilities.isPrivate(propertyDescriptor.getVisibility()))) {
+                 (forceField ||
+                  (Visibilities.isPrivate(propertyDescriptor.getVisibility()) &&
+                   isDefaultAccessor(propertyDescriptor.getGetter()) && isDefaultAccessor(propertyDescriptor.getSetter())))) {
             fieldAccessorKind = FieldAccessorKind.IN_CLASS_COMPANION;
         }
         else if ((syntheticBackingField &&
@@ -2005,6 +2011,10 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
         boolean skipPropertyAccessors;
 
         PropertyDescriptor originalPropertyDescriptor = DescriptorUtils.unwrapFakeOverride(propertyDescriptor);
+        boolean directAccessToGetter = couldUseDirectAccessToProperty(propertyDescriptor, true, isDelegatedProperty, context,
+                                                                              state.getShouldInlineConstVals());
+        boolean directAccessToSetter = couldUseDirectAccessToProperty(propertyDescriptor, false, isDelegatedProperty, context,
+                                                                              state.getShouldInlineConstVals());
 
         if (fieldAccessorKind == FieldAccessorKind.LATEINIT_INTRINSIC) {
             skipPropertyAccessors = !isPrivateProperty || context.getClassOrPackageParentContext() == backingFieldContext;
@@ -2016,8 +2026,9 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
             ownerDescriptor = propertyDescriptor;
         }
         else if (fieldAccessorKind == FieldAccessorKind.IN_CLASS_COMPANION || fieldAccessorKind == FieldAccessorKind.FIELD_FROM_LOCAL) {
-            boolean isInlinedConst = propertyDescriptor.isConst() && state.getShouldInlineConstVals();
+            // Do not use accessor 'access<property name>$cp' when the property can be accessed directly by its backing field.
-            skipPropertyAccessors = isInlinedConst || !isPrivateProperty || skipAccessorsForPrivateFieldInOuterClass;
+            skipPropertyAccessors = skipAccessorsForPrivateFieldInOuterClass ||
+                                    (directAccessToGetter && (!propertyDescriptor.isVar() || directAccessToSetter));
 
             if (!skipPropertyAccessors) {
                 //noinspection ConstantConditions
@@ -2038,23 +2049,22 @@ public class ExpressionCodegen extends KtVisitor<StackValue, StackValue> impleme
         }
 
         if (!skipPropertyAccessors) {
-            if (!couldUseDirectAccessToProperty(propertyDescriptor, true, isDelegatedProperty, context, state.getShouldInlineConstVals())) {
+            if (!directAccessToGetter || !directAccessToSetter) {
                 propertyDescriptor = context.getAccessorForSuperCallIfNeeded(propertyDescriptor, superCallTarget, state);
-
                 propertyDescriptor = context.accessibleDescriptor(propertyDescriptor, superCallTarget);
-
+                if (!isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
-                PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
+                    if (!directAccessToGetter) {
-                if (getter != null && !isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
+                        PropertyGetterDescriptor getter = propertyDescriptor.getGetter();
-                    callableGetter = typeMapper.mapToCallableMethod(getter, isSuper);
+                        if (getter != null) {
-                }
+                            callableGetter = typeMapper.mapToCallableMethod(getter, isSuper);
-            }
+                        }
-
+                    }
-            if (propertyDescriptor.isVar()) {
+                    if (propertyDescriptor.isVar() && !directAccessToSetter) {
-                PropertySetterDescriptor setter = propertyDescriptor.getSetter();
+                        PropertySetterDescriptor setter = propertyDescriptor.getSetter();
-                if (setter != null &&
+                        if (setter != null) {
-                    !couldUseDirectAccessToProperty(propertyDescriptor, false, isDelegatedProperty, context, state.getShouldInlineConstVals()) &&
+                            callableSetter = typeMapper.mapToCallableMethod(setter, isSuper);
-                    !isConstOrHasJvmFieldAnnotation(propertyDescriptor)) {
+                        }
-                    callableSetter = typeMapper.mapToCallableMethod(setter, isSuper);
+                    }
                 }
             }
         }

compiler/backend/src/org/jetbrains/kotlin/codegen/JvmCodegenUtil.java

@@ -118,11 +118,19 @@ public class JvmCodegenUtil {
                     !closure.isSuspend();
     }
 
-    private static boolean isCallInsideSameClassAsDeclared(@NotNull CallableMemberDescriptor descriptor, @NotNull CodegenContext context) {
+    private static boolean isCallInsideSameClassAsFieldRepresentingProperty(
+            @NotNull PropertyDescriptor descriptor,
+            @NotNull CodegenContext context
+    ) {
         boolean isFakeOverride = descriptor.getKind() == CallableMemberDescriptor.Kind.FAKE_OVERRIDE;
         boolean isDelegate = descriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION;
 
         DeclarationDescriptor containingDeclaration = descriptor.getContainingDeclaration().getOriginal();
+        if (JvmAbi.isPropertyWithBackingFieldInOuterClass(descriptor)) {
+            // For property with backed field, check if the access is done in the same class containing the backed field and
+            // not the class that declared the field.
+            containingDeclaration = containingDeclaration.getContainingDeclaration();
+        }
 
         return !isFakeOverride && !isDelegate &&
                (((context.hasThisDescriptor() && containingDeclaration == context.getThisDescriptor()) ||
@@ -188,12 +196,12 @@ public class JvmCodegenUtil {
         if (KotlinTypeMapper.isAccessor(property)) return false;
 
         CodegenContext context = contextBeforeInline.getFirstCrossInlineOrNonInlineContext();
-        // Inline functions can't use direct access because a field may not be visible at the call site
+        // Inline functions or inline classes can't use direct access because a field may not be visible at the call site
-        if (context.isInlineMethodContext()) {
+        if (context.isInlineMethodContext() || (context.getEnclosingClass() != null && context.getEnclosingClass().isInline())) {
             return false;
         }
 
-        if (!isCallInsideSameClassAsDeclared(property, context)) {
+        if (!isCallInsideSameClassAsFieldRepresentingProperty(property, context)) {
             if (!isDebuggerContext(context)) {
                 // Unless we are evaluating expression in debugger context, only properties of the same class can be directly accessed
                 return false;
@@ -218,9 +226,6 @@ public class JvmCodegenUtil {
         // Delegated and extension properties have no backing fields
         if (isDelegated || property.getExtensionReceiverParameter() != null) return false;
 
-        // Companion object properties cannot be accessed directly because their backing fields are stored in the containing class
-        if (DescriptorUtils.isCompanionObject(property.getContainingDeclaration())) return false;
-
         PropertyAccessorDescriptor accessor = forGetter ? property.getGetter() : property.getSetter();
 
         // If there's no accessor declared we can use direct access

compiler/backend/src/org/jetbrains/kotlin/codegen/PropertyCodegen.java

@@ -130,14 +130,20 @@ public class PropertyCodegen {
 
         genBackingFieldAndAnnotations(declaration, descriptor, false);
 
-        if (isAccessorNeeded(declaration, descriptor, getter)) {
+        boolean isDefaultGetterAndSetter = isDefaultAccessor(getter) && isDefaultAccessor(setter);
+
+        if (isAccessorNeeded(declaration, descriptor, getter, isDefaultGetterAndSetter)) {
             generateGetter(declaration, descriptor, getter);
         }
-        if (isAccessorNeeded(declaration, descriptor, setter)) {
+        if (isAccessorNeeded(declaration, descriptor, setter, isDefaultGetterAndSetter)) {
             generateSetter(declaration, descriptor, setter);
         }
     }
 
+    private static boolean isDefaultAccessor(@Nullable KtPropertyAccessor accessor) {
+        return accessor == null || !accessor.hasBody();
+    }
+
     private void genDestructuringDeclaration(
             @NotNull KtDestructuringDeclarationEntry entry,
             @NotNull PropertyDescriptor descriptor
@@ -194,11 +200,12 @@ public class PropertyCodegen {
     private boolean isAccessorNeeded(
             @Nullable KtProperty declaration,
             @NotNull PropertyDescriptor descriptor,
-            @Nullable KtPropertyAccessor accessor
+            @Nullable KtPropertyAccessor accessor,
+            boolean isDefaultGetterAndSetter
     ) {
         if (isConstOrHasJvmFieldAnnotation(descriptor)) return false;
 
-        boolean isDefaultAccessor = accessor == null || !accessor.hasBody();
+        boolean isDefaultAccessor = isDefaultAccessor(accessor);
 
         // Don't generate accessors for interface properties with default accessors in DefaultImpls
         if (kind == OwnerKind.DEFAULT_IMPLS && isDefaultAccessor) return false;
@@ -208,8 +215,16 @@ public class PropertyCodegen {
         // Delegated or extension properties can only be referenced via accessors
         if (declaration.hasDelegate() || declaration.getReceiverTypeReference() != null) return true;
 
-        // Companion object properties always should have accessors, because their backing fields are moved/copied to the outer class
+        // Companion object properties should have accessors for non-private properties because these properties can be referenced
-        if (isCompanionObject(descriptor.getContainingDeclaration())) return true;
+        // via getter/setter. But these accessors getter/setter are not required for private properties that have a default getter
+        // and setter, in this case, the property can always be accessed through the accessor 'access<property name>$cp' and avoid some
+        // useless indirection by using others accessors.
+        if (isCompanionObject(descriptor.getContainingDeclaration())) {
+            if (Visibilities.isPrivate(descriptor.getVisibility()) && isDefaultGetterAndSetter) {
+                return false;
+            }
+            return true;
+        }
 
         // Non-const properties from multifile classes have accessors regardless of visibility
         if (isNonConstTopLevelPropertyInMultifileClass(declaration, descriptor)) return true;

compiler/testData/asJava/lightClasses/compilationErrors/TraitClassObjectField.java

@@ -8,8 +8,6 @@ public interface TraitClassObjectField {
         public static final java.lang.String x = "";
         private static final java.lang.String y = "";
 
-        private final java.lang.String getY() { /* compiled code */ }
-
         private Companion() { /* compiled code */ }
     }
 }

compiler/testData/asJava/lightClasses/nullabilityAnnotations/ClassObjectField.java

@@ -10,8 +10,6 @@ public final class ClassObjectField {
         @org.jetbrains.annotations.Nullable
         public final java.lang.String getX() { /* compiled code */ }
 
-        private final java.lang.String getY() { /* compiled code */ }
-
         private Companion() { /* compiled code */ }
     }
 }

compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt

@@ -16,7 +16,9 @@ class A {
     fun `access$getBar$lp`(a: A): Int = 7
 
     companion object {
-        private var foo = 1;
+        private var foo = 1
+            // Custom getter is needed, otherwise no need to generate getY and setY
+            get() = field
 
         fun test() {
             {

compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.out

@@ -58,12 +58,12 @@ compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:18:15:
     fun `access$setFoo$p`(p: A.Companion, d: Int): Unit defined in A.Companion
     companion object {
               ^
-compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:28:9: error: platform declaration clash: The following declarations have the same JVM signature (access$getFoo$p(LA$Companion;)I):
+compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:30:9: error: platform declaration clash: The following declarations have the same JVM signature (access$getFoo$p(LA$Companion;)I):
     fun <get-foo>(): Int defined in A.Companion
     fun `access$getFoo$p`(p: A.Companion): Int defined in A.Companion
         fun `access$getFoo$p`(p: A.Companion): Int = 1
         ^
-compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:29:9: error: platform declaration clash: The following declarations have the same JVM signature (access$setFoo$p(LA$Companion;I)V):
+compiler/testData/cli/jvm/syntheticAccessorForPropertiesSignatureClash.kt:31:9: error: platform declaration clash: The following declarations have the same JVM signature (access$setFoo$p(LA$Companion;I)V):
     fun <set-foo>(<set-?>: Int): Unit defined in A.Companion
     fun `access$setFoo$p`(p: A.Companion, d: Int): Unit defined in A.Companion
         fun `access$setFoo$p`(p: A.Companion, d: Int) {}

compiler/testData/codegen/bytecodeText/companion/kt14258_1.kt

@@ -0,0 +1,16 @@
+// Checks that methods 'access$getMy$p', 'access$getMy$cp' and 'getMy' are not generated and
+// that backed field 'my' is directly used through a 'getstatic'
+
+class My {
+    companion object {
+        private val my: String = "OK"
+    }
+
+    fun getMyValue() = my
+}
+
+// 1 GETSTATIC My.my
+// 1 PUTSTATIC My.my
+// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
+// 0 INVOKESTATIC My.access\$getMy\$cp
+// 0 INVOKESPECIAL My\$Companion.getMy

compiler/testData/codegen/bytecodeText/companion/kt14258_2.kt

@@ -0,0 +1,20 @@
+// Checks that methods 'access$getMy$p' and 'getMy' are not generated and
+// that backed field 'my' is accessed through 'access$getMy$cp'
+
+class My {
+    companion object {
+        private val my: String = "OK"
+
+        fun test(): String {
+            // accessor is required because field is move to Foo
+            return my
+        }
+    }
+
+    fun getMyValue() = test()
+}
+
+// 1 GETSTATIC My.my
+// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
+// 1 INVOKESTATIC My.access\$getMy\$cp
+// 0 INVOKESPECIAL My\$Companion.getMy

compiler/testData/codegen/bytecodeText/companion/kt14258_3.kt

@@ -0,0 +1,28 @@
+// Checks that accessor methods are always used due to the overriding of the default setter of 'my' property.
+
+class My {
+    companion object {
+        private var my: String = "OK"
+            set(value) { field = value }
+    }
+
+    fun getMyValue(): String {
+        // INVOKESTATIC My$Companion.access$setMy$p
+        my = "Overriden value"
+        // GETSTATIC My.my
+        return my
+    }
+
+    // PUTSTATIC My.my into clinit
+    // PUTSTATIC My.my into 'access$setMy$cp'
+    // GETSTATIC My.my into 'access$getMy$cp'
+}
+
+// 2 GETSTATIC My.my
+// 2 PUTSTATIC My.my
+// 0 INVOKESTATIC My\$Companion.access\$getMy\$p
+// 1 INVOKESTATIC My\$Companion.access\$setMy\$p
+// 1 INVOKESTATIC My.access\$setMy\$cp
+// 1 INVOKESTATIC My.access\$getMy\$cp
+// 1 INVOKESPECIAL My\$Companion.getMy
+// 1 INVOKESPECIAL My\$Companion.setMy

compiler/testData/codegen/bytecodeText/companion/kt14258_4.kt

@@ -0,0 +1,23 @@
+// Checks that accessor 'I$Companion.access$getBar\$p' is always used because the property is kept
+// into the companion object.
+
+interface I {
+    companion object {
+        private var bar = "Companion Field from I"
+    }
+
+    fun test(): String {
+        // INVOKESTATIC I$Companion.access$setBar$p
+        bar = "New value"
+        // INVOKESTATIC I$Companion.access$getBar$p
+        return bar
+    }
+}
+
+// 1 GETSTATIC I\$Companion.bar
+// 2 PUTSTATIC I\$Companion.bar
+// 1 INVOKESTATIC I\$Companion.access\$getBar\$p
+// 1 INVOKESTATIC I\$Companion.access\$setBar\$p
+// 0 INVOKESTATIC I\$Companion.access\$setBar\$cp
+// 0 INVOKESPECIAL I\$Companion.getBar
+// 0 INVOKESPECIAL I\$Companion.setBar

compiler/testData/codegen/bytecodeText/companion/kt14258_5.kt

@@ -0,0 +1,19 @@
+// Checks that accessor are not used because property can be accessed directly.
+
+interface I {
+    companion object {
+        private var bar = "Companion Field from I"
+
+        fun test(): String {
+            bar = "New value"
+            return bar
+        }
+    }
+}
+
+// 1 GETSTATIC I\$Companion.bar
+// 2 PUTSTATIC I\$Companion.bar
+// 0 INVOKESTATIC I\$Companion.access\$getBar\$p
+// 0 INVOKESTATIC I\$Companion.access\$setBar\$cp
+// 0 INVOKESPECIAL I\$Companion.getBar
+// 0 INVOKESPECIAL I\$Companion.setBar

compiler/testData/codegen/bytecodeText/constProperty/accessorsForPrivateConstants.kt

@@ -9,9 +9,13 @@ class Foo {
     }
 
     fun foo() {
+        // Access to the property use getstatic on the backed field
         LOCAL_PRIVATE
+        // Access to the property requires an invokestatic
         OUTER_PRIVATE
     }
 }
 
-// 2 INVOKESTATIC
+// 1 INVOKESTATIC
+// 1 PUTSTATIC
+// 2 GETSTATIC

compiler/testData/codegen/bytecodeText/staticFields/classObjectSyntheticAccessor.kt

@@ -1,8 +0,0 @@
-class A {
-    companion object {
-        private var r: Int = 1;
-    }
-}
-// A and companion object constructor call
-// 3 ALOAD 0
-// 1 synthetic access\$getR

compiler/testData/writeFlags/function/classObjectPrivate/privateVal.kt

@@ -4,6 +4,8 @@ class Foo {
 
   companion object {
     private val test = "String"
+      // Custom getter is needed, otherwise no need to generate getTest
+      get() = field
   }
 }
 

compiler/testData/writeFlags/function/classObjectPrivate/privateVar.kt

@@ -4,6 +4,8 @@ class Foo {
 
   companion object {
     private var test = "String"
+      // Custom setter is needed, otherwise no need to generate setTest
+      set(v) { field = v }
   }
 }
 

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

@@ -853,6 +853,45 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
         }
     }
 
+    @TestMetadata("compiler/testData/codegen/bytecodeText/companion")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class Companion extends AbstractBytecodeTextTest {
+        public void testAllFilesPresentInCompanion() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/codegen/bytecodeText/companion"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("kt14258_1.kt")
+        public void testKt14258_1() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_1.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kt14258_2.kt")
+        public void testKt14258_2() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_2.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kt14258_3.kt")
+        public void testKt14258_3() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_3.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kt14258_4.kt")
+        public void testKt14258_4() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_4.kt");
+            doTest(fileName);
+        }
+
+        @TestMetadata("kt14258_5.kt")
+        public void testKt14258_5() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/companion/kt14258_5.kt");
+            doTest(fileName);
+        }
+    }
+
     @TestMetadata("compiler/testData/codegen/bytecodeText/conditions")
     @TestDataPath("$PROJECT_ROOT")
     @RunWith(JUnit3RunnerWithInners.class)
@@ -2601,12 +2640,6 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
             doTest(fileName);
         }
 
-        @TestMetadata("classObjectSyntheticAccessor.kt")
-        public void testClassObjectSyntheticAccessor() throws Exception {
-            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/staticFields/classObjectSyntheticAccessor.kt");
-            doTest(fileName);
-        }
-
         @TestMetadata("object.kt")
         public void testObject() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeText/staticFields/object.kt");