From 98cdc95cd07ee890b87fd03a52f870a741f0ea31 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Mon, 17 Jan 2022 14:18:58 +0300 Subject: [PATCH] FIR DFA: exit delegate expression only after accessor completion This commit fixes DFA problem (see test) when we accidentally try to merge incoming flows from member property (val ... by cached ...) and the following function. While completing lambda of buildList { ... } inside by cached we accidentally assume that delegate is already left and add the lambda into exitsFromCompletedPostponedAnonymousFunction. This commit fixes the problem by exiting delegate later, after completion. --- ...nosisCompilerFirTestdataTestGenerated.java | 6 ++ .../problems/WithValidityAssertion.fir.txt | 84 +++++++++++++++++ .../problems/WithValidityAssertion.kt | 66 +++++++++++++ .../runners/FirDiagnosticTestGenerated.java | 6 ++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++ .../FirDeclarationsResolveTransformer.kt | 94 +++++++++---------- 6 files changed, 214 insertions(+), 48 deletions(-) create mode 100644 compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.fir.txt create mode 100644 compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java index e1f957f159f..e66acee084e 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosisCompilerFirTestdataTestGenerated.java @@ -5884,6 +5884,12 @@ public class DiagnosisCompilerFirTestdataTestGenerated extends AbstractDiagnosis public void testWeakHashMap() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/weakHashMap.kt"); } + + @Test + @TestMetadata("WithValidityAssertion.kt") + public void testWithValidityAssertion() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt"); + } } @Nested diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.fir.txt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.fir.txt new file mode 100644 index 00000000000..3aff65c03ca --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.fir.txt @@ -0,0 +1,84 @@ +FILE: WithValidityAssertion.kt + public abstract interface ValidityToken : R|kotlin/Any| { + public abstract fun assertIsValid(): R|kotlin/Unit| + + } + public abstract interface ValidityTokenOwner : R|kotlin/Any| { + public abstract val token: R|ValidityToken| + public get(): R|ValidityToken| + + } + public final inline fun R|ValidityTokenOwner|.assertIsValid(): R|kotlin/Unit| { + this@R|/assertIsValid|.R|/ValidityTokenOwner.token|.R|/ValidityToken.assertIsValid|() + } + public final inline fun R|ValidityTokenOwner|.withValidityAssertion(action: R|() -> R|): R|R| { + this@R|/withValidityAssertion|.R|/assertIsValid|() + ^withValidityAssertion R|/action|.R|SubstitutionOverride|() + } + public final class ValidityAwareCachedValue : R|kotlin/properties/ReadOnlyProperty| { + public constructor(token: R|ValidityToken|, init: R|() -> T|): R|ValidityAwareCachedValue| { + super() + } + + private final val token: R|ValidityToken| = R|/token| + private get(): R|ValidityToken| + + private final val lazyValue: R|kotlin/Lazy| = R|kotlin/lazy|(Q|kotlin/LazyThreadSafetyMode|.R|kotlin/LazyThreadSafetyMode.PUBLICATION|, R|/init|) + private get(): R|kotlin/Lazy| + + @R|kotlin/Suppress|(names = vararg(String(UNCHECKED_CAST))) public final override operator fun getValue(thisRef: R|kotlin/Any|, property: R|kotlin/reflect/KProperty<*>|): R|T| { + this@R|/ValidityAwareCachedValue|.R|/ValidityAwareCachedValue.token|.R|/ValidityToken.assertIsValid|() + ^getValue this@R|/ValidityAwareCachedValue|.R|/ValidityAwareCachedValue.lazyValue|.R|SubstitutionOverride| + } + + } + internal final fun R|ValidityTokenOwner|.cached(init: R|() -> T|): R|ValidityAwareCachedValue| { + ^cached R|/ValidityAwareCachedValue.ValidityAwareCachedValue|(this@R|/cached|.R|/ValidityTokenOwner.token|, R|/init|) + } + public final typealias KtScopeNameFilter = R|(kotlin/String) -> kotlin/Boolean| + public abstract class KtFirNonStarImportingScope : R|ValidityTokenOwner| { + public constructor(firScope: R|FirScope|, builder: R|KtSymbolByFirBuilder|, token: R|ValidityToken|): R|KtFirNonStarImportingScope| { + super() + } + + private final val firScope: R|FirScope| = R|/firScope| + private get(): R|FirScope| + + private final val builder: R|KtSymbolByFirBuilder| = R|/builder| + private get(): R|KtSymbolByFirBuilder| + + public open override val token: R|ValidityToken| = R|/token| + public get(): R|ValidityToken| + + private final val imports: R|kotlin/collections/List|by this@R|/KtFirNonStarImportingScope|.R|/cached||>( = cached@fun (): R|kotlin/collections/List| { + ^ R|kotlin/collections/buildList|( = buildList@fun R|kotlin/collections/MutableList|.(): R|kotlin/Unit| { + this@R|/KtFirNonStarImportingScope|.R|/KtFirNonStarImportingScope.getCallableNames|().R|kotlin/collections/forEach|( = forEach@fun (it: R|kotlin/String|): R|kotlin/Unit| { + this@R|special/anonymous|.R|SubstitutionOverride|(R|/it|) + } + ) + } + ) + } + ) + private get(): R|kotlin/collections/List| { + ^ this@R|/KtFirNonStarImportingScope|.D|/KtFirNonStarImportingScope.imports|.R|SubstitutionOverride|(this@R|/KtFirNonStarImportingScope|, ::R|/KtFirNonStarImportingScope.imports|) + } + + public final fun getCallableSymbols(nameFilter: R|KtScopeNameFilter|): R|kotlin/sequences/Sequence| { + ^getCallableSymbols this@R|/KtFirNonStarImportingScope|.R|/withValidityAssertion||>( = withValidityAssertion@fun (): R|kotlin/sequences/Sequence| { + ^ this@R|/KtFirNonStarImportingScope|.R|/KtFirNonStarImportingScope.firScope|.R|/FirScope.getCallableSymbols|(this@R|/KtFirNonStarImportingScope|.R|/KtFirNonStarImportingScope.getCallableNames|().R|kotlin/collections/filter|(R|/nameFilter|), this@R|/KtFirNonStarImportingScope|.R|/KtFirNonStarImportingScope.builder|) + } + ) + } + + public abstract fun getCallableNames(): R|kotlin/collections/Set| + + } + public abstract interface FirScope : R|kotlin/Any| { + public abstract fun getCallableSymbols(callableNames: R|kotlin/collections/Collection|, builder: R|KtSymbolByFirBuilder|): R|kotlin/sequences/Sequence| + + } + public abstract interface KtCallableSymbol : R|kotlin/Any| { + } + public abstract interface KtSymbolByFirBuilder : R|kotlin/Any| { + } diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt new file mode 100644 index 00000000000..f687357a936 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt @@ -0,0 +1,66 @@ +import kotlin.reflect.KProperty +import kotlin.properties.ReadOnlyProperty + +interface ValidityToken { + fun assertIsValid() +} + +interface ValidityTokenOwner { + val token: ValidityToken +} + +inline fun ValidityTokenOwner.assertIsValid() { + token.assertIsValid() +} + +inline fun ValidityTokenOwner.withValidityAssertion(action: () -> R): R { + assertIsValid() + return action() +} + +class ValidityAwareCachedValue( + private val token: ValidityToken, + init: () -> T +) : ReadOnlyProperty { + private val lazyValue = lazy(LazyThreadSafetyMode.PUBLICATION, init) + + @Suppress("UNCHECKED_CAST") + override fun getValue(thisRef: Any, property: KProperty<*>): T { + token.assertIsValid() + return lazyValue.value + } +} + +internal fun ValidityTokenOwner.cached(init: () -> T) = ValidityAwareCachedValue(token, init) + +public typealias KtScopeNameFilter = (String) -> Boolean + +abstract class KtFirNonStarImportingScope( + private val firScope: FirScope, + private val builder: KtSymbolByFirBuilder, + override val token: ValidityToken, +) : ValidityTokenOwner { + private val imports: List by cached { + buildList { + getCallableNames().forEach { + add(it) + } + } + } + + fun getCallableSymbols(nameFilter: KtScopeNameFilter): Sequence = withValidityAssertion { + firScope.getCallableSymbols(getCallableNames().filter(nameFilter), builder) + } + + abstract fun getCallableNames(): Set +} + +interface FirScope { + fun getCallableSymbols(callableNames: Collection, builder: KtSymbolByFirBuilder): Sequence +} + +interface KtCallableSymbol + +interface KtSymbolByFirBuilder + + diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java index 9ca356b10fa..a25ce01ca53 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticTestGenerated.java @@ -5884,6 +5884,12 @@ public class FirDiagnosticTestGenerated extends AbstractFirDiagnosticTest { public void testWeakHashMap() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/weakHashMap.kt"); } + + @Test + @TestMetadata("WithValidityAssertion.kt") + public void testWithValidityAssertion() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt"); + } } @Nested diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java index d5bd6719bbf..58524dd0f2e 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirDiagnosticsWithLightTreeTestGenerated.java @@ -5884,6 +5884,12 @@ public class FirDiagnosticsWithLightTreeTestGenerated extends AbstractFirDiagnos public void testWeakHashMap() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/weakHashMap.kt"); } + + @Test + @TestMetadata("WithValidityAssertion.kt") + public void testWithValidityAssertion() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolveWithStdlib/problems/WithValidityAssertion.kt"); + } } @Nested diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt index 56cf0e1dcb4..248a38940d5 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt @@ -276,6 +276,8 @@ open class FirDeclarationsResolveTransformer(transformer: FirBodyResolveTransfor property.transformAccessors() val completedCalls = completeCandidates() + dataFlowAnalyzer.exitDelegateExpression() + val finalSubstitutor = createFinalSubstitutor() // Replace stub types with corresponding type variable types @@ -301,62 +303,58 @@ open class FirDeclarationsResolveTransformer(transformer: FirBodyResolveTransfor data: ResolutionMode, ): FirStatement { dataFlowAnalyzer.enterDelegateExpression() - try { - // First, resolve delegate expression in dependent context - val delegateExpression = - wrappedDelegateExpression.expression.transformSingle(transformer, ResolutionMode.ContextDependent) + // First, resolve delegate expression in dependent context + val delegateExpression = + wrappedDelegateExpression.expression.transformSingle(transformer, ResolutionMode.ContextDependent) - // Second, replace result type of delegate expression with stub type if delegate not yet resolved - if (delegateExpression is FirQualifiedAccess) { - val calleeReference = delegateExpression.calleeReference - if (calleeReference is FirNamedReferenceWithCandidate) { - val system = calleeReference.candidate.system - system.notFixedTypeVariables.forEach { - system.markPostponedVariable(it.value.typeVariable) - } - val typeVariableTypeToStubType = context.inferenceSession.createSyntheticStubTypes(system) - - val substitutor = createTypeSubstitutorByTypeConstructor(typeVariableTypeToStubType, session.typeContext) - val delegateExpressionTypeRef = delegateExpression.typeRef - val stubTypeSubstituted = substitutor.substituteOrNull(delegateExpressionTypeRef.coneType) - delegateExpression.replaceTypeRef(delegateExpressionTypeRef.withReplacedConeType(stubTypeSubstituted)) - } - } - - val provideDelegateCall = wrappedDelegateExpression.delegateProvider as FirFunctionCall - - // Resolve call for provideDelegate, without completion - provideDelegateCall.transformSingle(this, ResolutionMode.ContextIndependent) - - // If we got successful candidate for provideDelegate, let's select it - val provideDelegateCandidate = provideDelegateCall.candidate() - if (provideDelegateCandidate != null && provideDelegateCandidate.isSuccessful) { - val system = provideDelegateCandidate.system + // Second, replace result type of delegate expression with stub type if delegate not yet resolved + if (delegateExpression is FirQualifiedAccess) { + val calleeReference = delegateExpression.calleeReference + if (calleeReference is FirNamedReferenceWithCandidate) { + val system = calleeReference.candidate.system system.notFixedTypeVariables.forEach { system.markPostponedVariable(it.value.typeVariable) } val typeVariableTypeToStubType = context.inferenceSession.createSyntheticStubTypes(system) + val substitutor = createTypeSubstitutorByTypeConstructor(typeVariableTypeToStubType, session.typeContext) - - val stubTypeSubstituted = substitutor.substituteOrSelf(provideDelegateCandidate.substitutor.substituteOrSelf(components.typeFromCallee(provideDelegateCall).type)) - - provideDelegateCall.replaceTypeRef(provideDelegateCall.typeRef.resolvedTypeFromPrototype(stubTypeSubstituted)) - return provideDelegateCall + val delegateExpressionTypeRef = delegateExpression.typeRef + val stubTypeSubstituted = substitutor.substituteOrNull(delegateExpressionTypeRef.coneType) + delegateExpression.replaceTypeRef(delegateExpressionTypeRef.withReplacedConeType(stubTypeSubstituted)) } - - if (provideDelegateCall.calleeReference is FirResolvedNamedReference) { - return provideDelegateCall - } - - // Otherwise, rollback - (provideDelegateCall as? FirFunctionCall)?.let { dataFlowAnalyzer.dropSubgraphFromCall(it) } - - // Select delegate expression otherwise - return delegateExpression - .approximateIfIsIntegerConst() - } finally { - dataFlowAnalyzer.exitDelegateExpression() } + + val provideDelegateCall = wrappedDelegateExpression.delegateProvider as FirFunctionCall + + // Resolve call for provideDelegate, without completion + provideDelegateCall.transformSingle(this, ResolutionMode.ContextIndependent) + + // If we got successful candidate for provideDelegate, let's select it + val provideDelegateCandidate = provideDelegateCall.candidate() + if (provideDelegateCandidate != null && provideDelegateCandidate.isSuccessful) { + val system = provideDelegateCandidate.system + system.notFixedTypeVariables.forEach { + system.markPostponedVariable(it.value.typeVariable) + } + val typeVariableTypeToStubType = context.inferenceSession.createSyntheticStubTypes(system) + val substitutor = createTypeSubstitutorByTypeConstructor(typeVariableTypeToStubType, session.typeContext) + + val stubTypeSubstituted = substitutor.substituteOrSelf(provideDelegateCandidate.substitutor.substituteOrSelf(components.typeFromCallee(provideDelegateCall).type)) + + provideDelegateCall.replaceTypeRef(provideDelegateCall.typeRef.resolvedTypeFromPrototype(stubTypeSubstituted)) + return provideDelegateCall + } + + if (provideDelegateCall.calleeReference is FirResolvedNamedReference) { + return provideDelegateCall + } + + // Otherwise, rollback + (provideDelegateCall as? FirFunctionCall)?.let { dataFlowAnalyzer.dropSubgraphFromCall(it) } + + // Select delegate expression otherwise + return delegateExpression + .approximateIfIsIntegerConst() } private fun transformLocalVariable(variable: FirProperty): FirProperty {