From b57940a59beeab471e012c72706c0d4b9bee57f7 Mon Sep 17 00:00:00 2001 From: Roman Efremov Date: Thu, 17 Aug 2023 16:16:27 +0200 Subject: [PATCH] [FE1.0] Optimize search of expect class member in annotation checker In the following scenario, when we search corresponding expect member for actual `A.B`, we can skip checking compatibility of `B` scope. ``` class A { class B { fun foo() {} } } actual typealias AImpl = A ``` This is because: 1. Annotation checker runs no matter if found expect class is compatible or not. 2. Class always has at most one corresponding `expect` class (unlike for functions, which may have several overrides), so we are sure that we found the right member. ^KT-60668 ^KT-60936 --- ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++++ ...endMPPDiagnosticsWithPsiTestGenerated.java | 6 +++++ .../mpp/FirExpectActualMatchingContextImpl.kt | 1 + .../mpp/FirExpectActualResolver.kt | 7 +++++- .../IrExpectActualMatchingContext.kt | 1 + ...tractExpectActualAnnotationMatchChecker.kt | 9 ++++++- ...bstractExpectActualCompatibilityChecker.kt | 25 +++++++++++++------ .../calls/mpp/ExpectActualMatchingContext.kt | 1 + .../ClassicExpectActualMatchingContext.kt | 2 ++ .../multiplatform/ExpectedActualResolver.kt | 7 +++++- .../actualInnerClassMissingMember.fir.kt | 21 ++++++++++++++++ .../actualInnerClassMissingMember.kt | 21 ++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++++ 13 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.fir.kt create mode 100644 compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java index a3c0fc4b2eb..df0608137fc 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated.java @@ -687,6 +687,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst @TestMetadata("compiler/testData/diagnostics/tests/multiplatform/annotationMatching") @TestDataPath("$PROJECT_ROOT") public class AnnotationMatching { + @Test + @TestMetadata("actualInnerClassMissingMember.kt") + public void testActualInnerClassMissingMember() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt"); + } + @Test public void testAllFilesPresentInAnnotationMatching() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/annotationMatching"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), TargetBackend.JVM_IR, true); diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java index 4b4e96785fd..113dd96bc21 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendMPPDiagnosticsWithPsiTestGenerated.java @@ -687,6 +687,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi @TestMetadata("compiler/testData/diagnostics/tests/multiplatform/annotationMatching") @TestDataPath("$PROJECT_ROOT") public class AnnotationMatching { + @Test + @TestMetadata("actualInnerClassMissingMember.kt") + public void testActualInnerClassMissingMember() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt"); + } + @Test public void testAllFilesPresentInAnnotationMatching() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/annotationMatching"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), TargetBackend.JVM_IR, true); diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt index 5d433729a22..f6ca6b5bb0b 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualMatchingContextImpl.kt @@ -469,6 +469,7 @@ class FirExpectActualMatchingContextImpl private constructor( expectClass: RegularClassSymbolMarker, actualClass: RegularClassSymbolMarker, actualMember: DeclarationSymbolMarker, + checkClassScopesCompatibility: Boolean, ): Map, ExpectActualCompatibility<*>> { val mapping = actualClass.asSymbol().fir.memberExpectForActual return mapping?.get(actualMember to expectClass) ?: emptyMap() diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt index e2dd3c31895..349e8973d64 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/mpp/FirExpectActualResolver.kt @@ -90,7 +90,12 @@ object FirExpectActualResolver { is FirClassLikeSymbol<*> -> { val expectClassSymbol = useSiteSession.dependenciesSymbolProvider .getClassLikeSymbolByClassId(actualSymbol.classId) as? FirRegularClassSymbol ?: return null - val compatibility = AbstractExpectActualCompatibilityChecker.getClassifiersCompatibility(expectClassSymbol, actualSymbol, context) + val compatibility = AbstractExpectActualCompatibilityChecker.getClassifiersCompatibility( + expectClassSymbol, + actualSymbol, + checkClassScopesCompatibility = true, + context + ) mapOf(compatibility to listOf(expectClassSymbol)) } else -> null diff --git a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt index 7b033e00d1e..538182bca31 100644 --- a/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt +++ b/compiler/ir/backend.common/src/org/jetbrains/kotlin/backend/common/actualizer/IrExpectActualMatchingContext.kt @@ -517,5 +517,6 @@ internal abstract class IrExpectActualMatchingContext( expectClass: RegularClassSymbolMarker, actualClass: RegularClassSymbolMarker, actualMember: DeclarationSymbolMarker, + checkClassScopesCompatibility: Boolean, ): Map> = error("Should not be called") } diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt index f1a0e27d02f..20b55765a0e 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualAnnotationMatchChecker.kt @@ -154,7 +154,14 @@ object AbstractExpectActualAnnotationMatchChecker { if (skipCheckingAnnotationsOfActualClassMember(actualMember)) { continue } - val expectToCompatibilityMap = findPotentialExpectClassMembersForActual(expectClass, actualClass, actualMember) + val expectToCompatibilityMap = findPotentialExpectClassMembersForActual( + expectClass, actualClass, actualMember, + // Optimization: don't check class scopes, because: + // 1. Annotation checker runs no matter if found expect class is compatible or not. + // 2. Class always has at most one corresponding `expect` class (unlike for functions, which may have several overrides), + // so we are sure that we found the right member. + checkClassScopesCompatibility = false, + ) val expectMember = expectToCompatibilityMap.filter { it.value == ExpectActualCompatibility.Compatible }.keys.singleOrNull() // Check also incompatible members if only one is found ?: expectToCompatibilityMap.keys.singleOrNull() diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt index 16af103b58c..1a27593c876 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/AbstractExpectActualCompatibilityChecker.kt @@ -26,10 +26,11 @@ object AbstractExpectActualCompatibilityChecker { fun getClassifiersCompatibility( expectClassSymbol: RegularClassSymbolMarker, actualClassLikeSymbol: ClassLikeSymbolMarker, + checkClassScopesCompatibility: Boolean, context: ExpectActualMatchingContext, ): ExpectActualCompatibility { val result = with(context) { - getClassifiersCompatibility(expectClassSymbol, actualClassLikeSymbol, parentSubstitutor = null) + getClassifiersCompatibility(expectClassSymbol, actualClassLikeSymbol, parentSubstitutor = null, checkClassScopesCompatibility) } @Suppress("UNCHECKED_CAST") return result as ExpectActualCompatibility @@ -62,7 +63,8 @@ object AbstractExpectActualCompatibilityChecker { substitutor = null, expectClassSymbol = null, actualClassSymbol = null, - unfulfilled = null + unfulfilled = null, + checkClassScopesCompatibility = true, ) } } @@ -73,7 +75,8 @@ object AbstractExpectActualCompatibilityChecker { expectClassSymbol: RegularClassSymbolMarker, actualClassLikeSymbol: ClassLikeSymbolMarker, parentSubstitutor: TypeSubstitutorMarker?, - ): ExpectActualCompatibility<*> = getClassifiersIncompatibility(expectClassSymbol, actualClassLikeSymbol, parentSubstitutor) + checkClassScopes: Boolean, + ): ExpectActualCompatibility<*> = getClassifiersIncompatibility(expectClassSymbol, actualClassLikeSymbol, parentSubstitutor, checkClassScopes) ?: ExpectActualCompatibility.Compatible context(ExpectActualMatchingContext<*>) @@ -81,7 +84,8 @@ object AbstractExpectActualCompatibilityChecker { private fun getClassifiersIncompatibility( expectClassSymbol: RegularClassSymbolMarker, actualClassLikeSymbol: ClassLikeSymbolMarker, - parentSubstitutor: TypeSubstitutorMarker? + parentSubstitutor: TypeSubstitutorMarker?, + checkClassScopesCompatibility: Boolean, ): ExpectActualCompatibility.Incompatible.WeakIncompatible<*>? { // Can't check FQ names here because nested expected class may be implemented via actual typealias's expansion with the other FQ name require(expectClassSymbol.name == actualClassLikeSymbol.name) { @@ -136,7 +140,9 @@ object AbstractExpectActualCompatibilityChecker { return Incompatible.Supertypes } - getClassScopesIncompatibility(expectClassSymbol, actualClass, substitutor)?.let { return it } + if (checkClassScopesCompatibility) { + getClassScopesIncompatibility(expectClassSymbol, actualClass, substitutor)?.let { return it } + } return null } @@ -211,7 +217,8 @@ object AbstractExpectActualCompatibilityChecker { substitutor, expectClassSymbol, actualClassSymbol, - unfulfilled + unfulfilled, + checkClassScopesCompatibility = true, ) } @@ -236,7 +243,8 @@ object AbstractExpectActualCompatibilityChecker { substitutor: TypeSubstitutorMarker?, expectClassSymbol: RegularClassSymbolMarker?, actualClassSymbol: RegularClassSymbolMarker?, - unfulfilled: MutableList, List>>>? + unfulfilled: MutableList, List>>>?, + checkClassScopesCompatibility: Boolean, ) { val mapping = actualMembers.keysToMap { actualMember -> when (expectMember) { @@ -253,7 +261,8 @@ object AbstractExpectActualCompatibilityChecker { getClassifiersCompatibility( expectMember, actualMember as ClassLikeSymbolMarker, - parentSubstitutor + parentSubstitutor, + checkClassScopesCompatibility, ) } else -> error("Unsupported declaration: $expectMember ($actualMembers)") diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt index df69d5c3d72..b790ee5d452 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/mpp/ExpectActualMatchingContext.kt @@ -210,5 +210,6 @@ interface ExpectActualMatchingContext : TypeSystemC expectClass: RegularClassSymbolMarker, actualClass: RegularClassSymbolMarker, actualMember: DeclarationSymbolMarker, + checkClassScopesCompatibility: Boolean, ): Map> } diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ClassicExpectActualMatchingContext.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ClassicExpectActualMatchingContext.kt index 817b131402d..b35ff1a079e 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ClassicExpectActualMatchingContext.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ClassicExpectActualMatchingContext.kt @@ -396,11 +396,13 @@ class ClassicExpectActualMatchingContext( expectClass: RegularClassSymbolMarker, actualClass: RegularClassSymbolMarker, actualMember: DeclarationSymbolMarker, + checkClassScopesCompatibility: Boolean, ): Map> { val compatibilityToExpects = ExpectedActualResolver.findExpectForActualClassMember( actualMember as MemberDescriptor, actualClass as ClassDescriptor, expectClass as ClassDescriptor, + checkClassScopesCompatibility, this, ) return buildMap { diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt index 8c83d049a44..b50398523da 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/multiplatform/ExpectedActualResolver.kt @@ -46,6 +46,7 @@ object ExpectedActualResolver { AbstractExpectActualCompatibilityChecker.getClassifiersCompatibility( expected, actual, + checkClassScopesCompatibility = true, context ) } @@ -58,6 +59,7 @@ object ExpectedActualResolver { actual: MemberDescriptor, actualClass: ClassDescriptor, expectClass: ClassDescriptor, + checkClassScopesCompatibility: Boolean, context: ClassicExpectActualMatchingContext, ): Map, List> { val candidates = with(context) { @@ -76,6 +78,7 @@ object ExpectedActualResolver { matchActualClassAgainstPotentialExpects( actual, candidates.filterIsInstance(), + checkClassScopesCompatibility, context ) } @@ -110,7 +113,7 @@ object ExpectedActualResolver { } is ClassifierDescriptorWithTypeParameters -> { val candidates = context.findClassifiersFromModule(actual.classId, actual.module, moduleFilter) - matchActualClassAgainstPotentialExpects(actual, candidates, context) + matchActualClassAgainstPotentialExpects(actual, candidates, checkClassScopesCompatibility = true, context) } else -> null } @@ -158,6 +161,7 @@ object ExpectedActualResolver { private fun matchActualClassAgainstPotentialExpects( actual: ClassifierDescriptorWithTypeParameters, candidates: Collection, + checkClassScopesCompatibility: Boolean, context: ClassicExpectActualMatchingContext, ): Map, List> { return candidates.filter { declaration -> @@ -166,6 +170,7 @@ object ExpectedActualResolver { AbstractExpectActualCompatibilityChecker.getClassifiersCompatibility( expected as ClassDescriptor, actual, + checkClassScopesCompatibility, context ) } diff --git a/compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.fir.kt b/compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.fir.kt new file mode 100644 index 00000000000..de637d03ea4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.fir.kt @@ -0,0 +1,21 @@ +// MODULE: m1-common +// FILE: common.kt +annotation class Ann + +expect class A { + class B { + @Ann + fun foo() + fun missingOnActual() + } +} + +// MODULE: m1-jvm()()(m1-common) +// FILE: jvm.kt +class AImpl { + class B { + fun foo() {} + } +} + +actual typealias A = AImpl diff --git a/compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt b/compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt new file mode 100644 index 00000000000..a6dc9644c48 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt @@ -0,0 +1,21 @@ +// MODULE: m1-common +// FILE: common.kt +annotation class Ann + +expect class A { + class B { + @Ann + fun foo() + fun missingOnActual() + } +} + +// MODULE: m1-jvm()()(m1-common) +// FILE: jvm.kt +class AImpl { + class B { + fun foo() {} + } +} + +actual typealias A = AImpl diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index b0675f35bcb..29503239e0a 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -23456,6 +23456,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { @TestMetadata("compiler/testData/diagnostics/tests/multiplatform/annotationMatching") @TestDataPath("$PROJECT_ROOT") public class AnnotationMatching { + @Test + @TestMetadata("actualInnerClassMissingMember.kt") + public void testActualInnerClassMissingMember() throws Exception { + runTest("compiler/testData/diagnostics/tests/multiplatform/annotationMatching/actualInnerClassMissingMember.kt"); + } + @Test public void testAllFilesPresentInAnnotationMatching() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/multiplatform/annotationMatching"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true);