[FIR] 1/2 match and check expect fake-overrides vs actuals

^KT-63550 Fixed
Review: https://jetbrains.team/p/kt/reviews/13094/timeline

Now it's required to create a new ScopeSession when searching for expect
members for actuals. If you keep reusing actualScopeSession then members
declared in platform may "slip into" the search results, resulting an
incorrect expect for actual (e.g. it happens in
supertypeIsExpectActual_covariantOverrideOfInjectedFromSuper_transitiveSubstitutionFakeOverride.fir.kt)

I suppose that it always has been a bug that we reused
actualScopeSession because we were mixing actualScopeSession and
expect FirSession (which is a bad idea), but we simply didn't have cases
where this bug could be observed. Now after we started matching
fake-overrides, we have such cases.

But creating a new ScopeSession every time is a suboptimal solution. We
need to design a scope caching KT-63773
This commit is contained in:
Nikita Bobko
2023-11-15 19:25:18 +01:00
committed by teamcity
parent 5aa0475aa7
commit a1ce8ac175
58 changed files with 182 additions and 69 deletions
@@ -68,6 +68,12 @@ public class FirOldFrontendMPPDiagnosticsWithLightTreeTestGenerated extends Abst
runTest("compiler/testData/diagnostics/tests/multiplatform/actualFakeOverride_transitiveFakeOverrides_incompatible.kt");
}
@Test
@TestMetadata("actualTypealiasForNotExpectClass.kt")
public void testActualTypealiasForNotExpectClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/multiplatform/actualTypealiasForNotExpectClass.kt");
}
@Test
@TestMetadata("actualTypealiasToSpecialAnnotation.kt")
public void testActualTypealiasToSpecialAnnotation() throws Exception {
@@ -68,6 +68,12 @@ public class FirOldFrontendMPPDiagnosticsWithPsiTestGenerated extends AbstractFi
runTest("compiler/testData/diagnostics/tests/multiplatform/actualFakeOverride_transitiveFakeOverrides_incompatible.kt");
}
@Test
@TestMetadata("actualTypealiasForNotExpectClass.kt")
public void testActualTypealiasForNotExpectClass() throws Exception {
runTest("compiler/testData/diagnostics/tests/multiplatform/actualTypealiasForNotExpectClass.kt");
}
@Test
@TestMetadata("actualTypealiasToSpecialAnnotation.kt")
public void testActualTypealiasToSpecialAnnotation() throws Exception {
@@ -20,13 +20,14 @@ interface FirExpectActualMatchingContext : ExpectActualMatchingContext<FirBasedS
session: FirSession = moduleData.session,
): Collection<FirConstructorSymbol>
val expectScopeSession: ScopeSession
override fun RegularClassSymbolMarker.getMembersForExpectClass(name: Name): List<FirCallableSymbol<*>>
}
interface FirExpectActualMatchingContextFactory : FirSessionComponent {
fun create(
session: FirSession,
scopeSession: ScopeSession,
actualSession: FirSession,
actualScopeSession: ScopeSession,
allowedWritingMemberExpectForActualMapping: Boolean = false,
): FirExpectActualMatchingContext
}
@@ -41,11 +41,11 @@ class FirExpectActualMatcherProcessor(
*/
open class FirExpectActualMatcherTransformer(
final override val session: FirSession,
private val scopeSession: ScopeSession,
private val actualScopeSession: ScopeSession,
) : FirAbstractTreeTransformer<Nothing?>(FirResolvePhase.EXPECT_ACTUAL_MATCHING) {
private val expectActualMatchingContext = session.expectActualMatchingContextFactory.create(
session, scopeSession,
session, actualScopeSession,
allowedWritingMemberExpectForActualMapping = true,
)
@@ -102,7 +102,6 @@ open class FirExpectActualMatcherTransformer(
val expectForActualData = FirExpectActualResolver.findExpectForActual(
actualSymbol,
session,
scopeSession,
expectActualMatchingContext,
)
memberDeclaration.expectForActual = expectForActualData
@@ -40,7 +40,7 @@ import org.jetbrains.kotlin.utils.zipIfSizesAreEqual
class FirExpectActualMatchingContextImpl private constructor(
private val actualSession: FirSession,
private val scopeSession: ScopeSession,
private val actualScopeSession: ScopeSession,
private val allowedWritingMemberExpectForActualMapping: Boolean,
) : FirExpectActualMatchingContext, TypeSystemContext by actualSession.typeContext {
override val shouldCheckAbsenceOfDefaultParamsInActual: Boolean
@@ -52,6 +52,11 @@ class FirExpectActualMatchingContextImpl private constructor(
override val allowTransitiveSupertypesActualization: Boolean
get() = true
override val expectScopeSession: ScopeSession
// todo KT-63773 design a way for managing scope sessions for common scopes during matching.
// Right now we create a new session every time
get() = ScopeSession()
private fun DeclarationSymbolMarker.asSymbol(): FirBasedSymbol<*> = this as FirBasedSymbol<*>
private fun CallableSymbolMarker.asSymbol(): FirCallableSymbol<*> = this as FirCallableSymbol<*>
private fun FunctionSymbolMarker.asSymbol(): FirFunctionSymbol<*> = this as FirFunctionSymbol<*>
@@ -170,7 +175,7 @@ class FirExpectActualMatchingContextImpl private constructor(
val scope = symbol.defaultType().scope(
useSiteSession = session,
scopeSession,
if (isActualDeclaration) actualScopeSession else expectScopeSession,
CallableCopyTypeCalculator.DoNothing,
requiredMembersPhase = FirResolvePhase.STATUS,
) ?: return emptyList()
@@ -196,7 +201,7 @@ class FirExpectActualMatchingContextImpl private constructor(
val symbol = asSymbol()
val scope = symbol.defaultType().scope(
useSiteSession = symbol.moduleData.session,
scopeSession,
expectScopeSession,
CallableCopyTypeCalculator.DoNothing,
requiredMembersPhase = FirResolvePhase.STATUS,
) ?: return emptyList()
@@ -259,7 +264,7 @@ class FirExpectActualMatchingContextImpl private constructor(
val session = symbol.moduleData.session
val containingClass = symbol.containingClassLookupTag()?.toFirRegularClassSymbol(session)
?: return sequenceOf(symbol)
(sequenceOf(symbol) + symbol.overriddenFunctions(containingClass, session, scopeSession).asSequence())
(sequenceOf(symbol) + symbol.overriddenFunctions(containingClass, session, actualScopeSession).asSequence())
// Tests work even if you don't filter out fake-overrides. Filtering fake-overrides is needed because
// the returned descriptors are compared by `equals`. And `equals` for fake-overrides is weird.
// I didn't manage to invent a test that would check this condition
@@ -354,7 +359,7 @@ class FirExpectActualMatchingContextImpl private constructor(
override fun RegularClassSymbolMarker.isNotSamInterface(): Boolean {
val type = asSymbol().defaultType()
val isSam = FirSamResolver(actualSession, scopeSession).isSamType(type)
val isSam = FirSamResolver(actualSession, actualScopeSession).isSamType(type)
return !isSam
}
@@ -613,9 +618,9 @@ class FirExpectActualMatchingContextImpl private constructor(
object Factory : FirExpectActualMatchingContextFactory {
override fun create(
session: FirSession, scopeSession: ScopeSession,
actualSession: FirSession, actualScopeSession: ScopeSession,
allowedWritingMemberExpectForActualMapping: Boolean,
): FirExpectActualMatchingContextImpl =
FirExpectActualMatchingContextImpl(session, scopeSession, allowedWritingMemberExpectForActualMapping)
FirExpectActualMatchingContextImpl(actualSession, actualScopeSession, allowedWritingMemberExpectForActualMapping)
}
}
@@ -9,6 +9,7 @@ import org.jetbrains.kotlin.fir.FirExpectActualMatchingContext
import org.jetbrains.kotlin.fir.FirSession
import org.jetbrains.kotlin.fir.declarations.ExpectForActualMatchingData
import org.jetbrains.kotlin.fir.declarations.fullyExpandedClass
import org.jetbrains.kotlin.fir.declarations.utils.isExpect
import org.jetbrains.kotlin.fir.resolve.ScopeSession
import org.jetbrains.kotlin.fir.resolve.providers.dependenciesSymbolProvider
import org.jetbrains.kotlin.fir.resolve.providers.symbolProvider
@@ -26,7 +27,6 @@ object FirExpectActualResolver {
fun findExpectForActual(
actualSymbol: FirBasedSymbol<*>,
useSiteSession: FirSession,
scopeSession: ScopeSession,
context: FirExpectActualMatchingContext,
): ExpectForActualMatchingData {
with(context) {
@@ -45,7 +45,7 @@ object FirExpectActualResolver {
?.fullyExpandedClass(useSiteSession)
when (actualSymbol) {
is FirConstructorSymbol -> expectContainingClass?.getConstructors(scopeSession)
is FirConstructorSymbol -> expectContainingClass?.getConstructors(expectScopeSession)
else -> expectContainingClass?.getMembersForExpectClass(actualSymbol.name)
}.orEmpty()
}
@@ -59,7 +59,7 @@ object FirExpectActualResolver {
}
}
candidates.filter { expectSymbol ->
actualSymbol != expectSymbol && expectSymbol.isExpect
actualSymbol != expectSymbol && (expectContainingClass != null /*match fake overrides*/ || expectSymbol.isExpect)
}.groupBy { expectDeclaration ->
AbstractExpectActualMatcher.getCallablesMatchingCompatibility(
expectDeclaration,
@@ -79,8 +79,12 @@ object FirExpectActualResolver {
is FirClassLikeSymbol<*> -> {
val expectClassSymbol = useSiteSession.dependenciesSymbolProvider
.getClassLikeSymbolByClassId(actualSymbol.classId) as? FirRegularClassSymbol ?: return emptyMap()
val compatibility = AbstractExpectActualMatcher.matchClassifiers(expectClassSymbol, actualSymbol, context)
mapOf(compatibility to listOf(expectClassSymbol))
if (expectClassSymbol.isExpect) {
val compatibility = AbstractExpectActualMatcher.matchClassifiers(expectClassSymbol, actualSymbol, context)
mapOf(compatibility to listOf(expectClassSymbol))
} else {
emptyMap()
}
}
else -> emptyMap()
}