From 7e2f15f532a5e3dff7035c2009064647c3e107ed Mon Sep 17 00:00:00 2001 From: Tianyu Geng Date: Mon, 26 Jul 2021 21:44:46 -0700 Subject: [PATCH] FIR DFA: fix logic clear aliasing The logic should clear back aliases as well. To ensure all back aliases don't lose any information, statements on the original variable are copied over to its aliases. --- ...irOldFrontendDiagnosticsTestGenerated.java | 6 ++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 ++ .../kotlin/fir/resolve/calls/Arguments.kt | 2 +- .../fir/resolve/dfa/PersistentLogicSystem.kt | 63 ++++++++++++++++-- .../FirDeclarationsResolveTransformer.kt | 1 + .../kotlin/fir/types/FirTypeUtils.kt | 9 +++ .../smartCasts/variables/aliasing.fir.kt | 65 +++++++++++++++++++ .../tests/smartCasts/variables/aliasing.kt | 65 +++++++++++++++++++ .../tests/smartCasts/variables/aliasing.txt | 4 ++ .../varnotnull/boundInitializerWrong.fir.kt | 2 +- .../varnotnull/plusplusMinusminus.fir.kt | 2 +- .../test/runners/DiagnosticTestGenerated.java | 6 ++ ...CompilerTestFE10TestdataTestGenerated.java | 6 ++ .../fir/components/KtFirSmartcastProvider.kt | 5 +- 14 files changed, 233 insertions(+), 9 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.fir.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.txt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index a84498b6402..c91e6bf1a19 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -28217,6 +28217,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti runTest("compiler/testData/diagnostics/tests/smartCasts/variables/accessorAndFunction.kt"); } + @Test + @TestMetadata("aliasing.kt") + public void testAliasing() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt"); + } + @Test public void testAllFilesPresentInVariables() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/variables"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index 9354c4d7d27..1038d9f9612 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -28217,6 +28217,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac runTest("compiler/testData/diagnostics/tests/smartCasts/variables/accessorAndFunction.kt"); } + @Test + @TestMetadata("aliasing.kt") + public void testAliasing() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt"); + } + @Test public void testAllFilesPresentInVariables() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/variables"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt index 4462128c7a6..fd4f80b919e 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt @@ -396,7 +396,7 @@ private fun checkApplicabilityForArgumentType( if (!csBuilder.addSubtypeConstraintIfCompatible(argumentType, expectedType, position)) { val smartcastExpression = argument as? FirExpressionWithSmartcast - if (smartcastExpression != null && smartcastExpression.smartcastStability != SmartcastStability.STABLE_VALUE) { + if (smartcastExpression != null && !smartcastExpression.isStable) { val unstableType = smartcastExpression.smartcastType.coneType if (csBuilder.addSubtypeConstraintIfCompatible(unstableType, expectedType, position)) { sink.reportDiagnostic(UnstableSmartCast(smartcastExpression, expectedType)) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt index 8965f40f959..eef6beaafea 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt @@ -202,13 +202,68 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste ) } + /** + * For example, consider `var b = a`. In this case, `b` is an alias of `a`. In other words, `a` is the original variable of `b`. + * + * For back alias, consider the following. + * ``` + * var b = a + * var c = b + * ``` + * Here, if the current `alias` references `b`, `c` is a back alias of `b`. So if one calls this method with `b`, we must also + * remove aliasing between `b` and `c`. But before removing aliasing, we need to copy any statements that apply to `b` to `c`. + */ override fun removeLocalVariableAlias(flow: PersistentFlow, alias: RealVariable) { - val original = flow.directAliasMap[alias]?.variable ?: return - flow.directAliasMap = flow.directAliasMap.remove(alias) - val variables = flow.backwardsAliasMap.getValue(original) - flow.backwardsAliasMap = flow.backwardsAliasMap.put(original, variables - alias) + val backAliases = flow.backwardsAliasMap[alias] ?: emptyList() + for (backAlias in backAliases) { + flow.logicStatements[alias]?.let { it -> + val newStatements = it.map { it.replaceVariable(alias, backAlias) } + val replacedStatements = + flow.logicStatements[backAlias]?.let { existing -> existing + newStatements } ?: newStatements.toPersistentList() + flow.logicStatements = flow.logicStatements.put(backAlias, replacedStatements) + } + flow.approvedTypeStatements[alias]?.let { it -> + val newStatements = it.replaceVariable(alias, backAlias) + val replacedStatements = + flow.approvedTypeStatements[backAlias]?.let { existing -> existing + newStatements } ?: newStatements + flow.approvedTypeStatements = flow.approvedTypeStatements.put(backAlias, replacedStatements.toPersistent()) + } + flow.approvedTypeStatementsDiff[alias]?.let { it -> + val newStatements = it.replaceVariable(alias, backAlias) + val replacedStatements = + flow.approvedTypeStatementsDiff[backAlias]?.let { existing -> existing + newStatements } ?: newStatements + flow.approvedTypeStatementsDiff = flow.approvedTypeStatementsDiff.put(backAlias, replacedStatements.toPersistent()) + } + } + + val original = flow.directAliasMap[alias]?.variable + if (original != null) { + flow.directAliasMap = flow.directAliasMap.remove(alias) + val variables = flow.backwardsAliasMap.getValue(original) + flow.backwardsAliasMap = flow.backwardsAliasMap.put(original, variables - alias) + } + flow.backwardsAliasMap = flow.backwardsAliasMap.remove(alias) + for (backAlias in backAliases) { + flow.directAliasMap = flow.directAliasMap.remove(backAlias) + } } + private fun Implication.replaceVariable(from: RealVariable, to: RealVariable): Implication { + return Implication(condition.replaceVariable(from, to), effect.replaceVariable(from, to)) + } + + private fun > Statement.replaceVariable(from: RealVariable, to: RealVariable): T { + val statement = when (this) { + is OperationStatement -> if (variable == from) copy(variable = to) else this + is PersistentTypeStatement -> if (variable == from) copy(variable = to) else this + is MutableTypeStatement -> if (variable == from) MutableTypeStatement(to, exactType, exactNotType) else this + else -> throw IllegalArgumentException("unknown type of statement $this") + } + @Suppress("UNCHECKED_CAST") + return statement as T + } + + @OptIn(DfaInternals::class) private fun PersistentFlow.getApprovedTypeStatements(variable: RealVariable): MutableTypeStatement { var flow = this 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 7a7bb6ad876..1480b48bb40 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 @@ -882,4 +882,5 @@ open class FirDeclarationsResolveTransformer(transformer: FirBodyResolveTransfor private val FirFunction.bodyResolved: Boolean get() = body !is FirLazyBlock && body?.typeRef is FirResolvedTypeRef + } diff --git a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/types/FirTypeUtils.kt b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/types/FirTypeUtils.kt index e2d7352370a..8657e05dc19 100644 --- a/compiler/fir/tree/src/org/jetbrains/kotlin/fir/types/FirTypeUtils.kt +++ b/compiler/fir/tree/src/org/jetbrains/kotlin/fir/types/FirTypeUtils.kt @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.builtins.functions.FunctionClassKind import org.jetbrains.kotlin.fir.expressions.FirAnnotationCall import org.jetbrains.kotlin.fir.expressions.FirConstExpression import org.jetbrains.kotlin.fir.expressions.FirExpression +import org.jetbrains.kotlin.fir.expressions.FirExpressionWithSmartcast import org.jetbrains.kotlin.fir.render import org.jetbrains.kotlin.fir.types.impl.FirImplicitBuiltinTypeRef import org.jetbrains.kotlin.name.ClassId @@ -51,6 +52,14 @@ val FirExpression.isNullLiteral: Boolean this.value == null && this.source != null +@OptIn(ExperimentalContracts::class) +fun FirExpression.isStableSmartcast(): Boolean { + contract { + returns(true) implies (this@isStableSmartcast is FirExpressionWithSmartcast) + } + return this is FirExpressionWithSmartcast && this.isStable +} + private val FirTypeRef.classLikeTypeOrNull: ConeClassLikeType? get() = when (this) { is FirImplicitBuiltinTypeRef -> type diff --git a/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.fir.kt new file mode 100644 index 00000000000..3dc0b517806 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.fir.kt @@ -0,0 +1,65 @@ +fun test() { + var a: Any? = null + var b = a + var c = b + // Now both `b` and `c` are aliases of `a`. + + if (a is String) { + b.length // OK + c.length // OK + } + if (b is String) { + a.length // OK + c.length // OK + } + if (c is String) { + a.length // OK + b.length // OK + } + + b = 3 // break `b` -> `a` + if (a is String) { + b.length // error + c.length // OK, since `c` is aliased to `a` + } + if (b is String) { + a.length // error + c.length // error + } + if (c is String) { + a.length // OK, since `c` is alised to `a` + b.length // error + } + + a = 2 // break `c` -> `a` + if (a is String) { + b.length // error + c.length // error + } + if (b is String) { + a.length // error + c.length // error + } + if (c is String) { + a.length // error + b.length // error + } + + c = b // create aliasing `c` -> `b` + c.unaryPlus() // OK due to aliasing + b = "" + c.unaryPlus() // OK since `c` should carry all typing information that was on `b` before `b = ""`. + + c = "" + c.length // OK + c.unaryPlus() // error +} + +fun test2() { + var a: Any? = null + a = "" + var b = a + a = 3 + b.length // OK + b.unaryPlus() // error +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt b/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt new file mode 100644 index 00000000000..6b13dcf26dd --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt @@ -0,0 +1,65 @@ +fun test() { + var a: Any? = null + var b = a + var c = b + // Now both `b` and `c` are aliases of `a`. + + if (a is String) { + b.length // OK + c.length // OK + } + if (b is String) { + a.length // OK + c.length // OK + } + if (c is String) { + a.length // OK + b.length // OK + } + + b = 3 // break `b` -> `a` + if (a is String) { + b.length // error + c.length // OK, since `c` is aliased to `a` + } + if (b is String) { + a.length // error + c.length // error + } + if (c is String) { + a.length // OK, since `c` is alised to `a` + b.length // error + } + + a = 2 // break `c` -> `a` + if (a is String) { + b.length // error + c.length // error + } + if (b is String) { + a.length // error + c.length // error + } + if (c is String) { + a.length // error + b.length // error + } + + c = b // create aliasing `c` -> `b` + c.unaryPlus() // OK due to aliasing + b = "" + c.unaryPlus() // OK since `c` should carry all typing information that was on `b` before `b = ""`. + + c = "" + c.length // OK + c.unaryPlus() // error +} + +fun test2() { + var a: Any? = null + a = "" + var b = a + a = 3 + b.length // OK + b.unaryPlus() // error +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.txt b/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.txt new file mode 100644 index 00000000000..6309e83aef4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.txt @@ -0,0 +1,4 @@ +package + +public fun test(): kotlin.Unit +public fun test2(): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/smartCasts/varnotnull/boundInitializerWrong.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/varnotnull/boundInitializerWrong.fir.kt index 73fd3227a42..d20640771cd 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/varnotnull/boundInitializerWrong.fir.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/varnotnull/boundInitializerWrong.fir.kt @@ -5,7 +5,7 @@ fun foo() { val y = x x = null if (y != null) { - x.hashCode() + x.hashCode() } } diff --git a/compiler/testData/diagnostics/tests/smartCasts/varnotnull/plusplusMinusminus.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/varnotnull/plusplusMinusminus.fir.kt index a4e834e56b9..97923e11e59 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/varnotnull/plusplusMinusminus.fir.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/varnotnull/plusplusMinusminus.fir.kt @@ -11,7 +11,7 @@ operator fun Long?.inc() = this?.let { it + 1 } fun bar(arg: Long?): Long { var i = arg if (i++ == 5L) { - return i-- + i + return i-- + i } if (i++ == 7L) { return i++ + i 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 cde4d2a6ce6..856b56f2964 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 @@ -28307,6 +28307,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/smartCasts/variables/accessorAndFunction.kt"); } + @Test + @TestMetadata("aliasing.kt") + public void testAliasing() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt"); + } + @Test public void testAllFilesPresentInVariables() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/variables"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); diff --git a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index e40e8bf3bbc..3676b5cb724 100644 --- a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -28217,6 +28217,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/smartCasts/variables/accessorAndFunction.kt"); } + @Test + @TestMetadata("aliasing.kt") + public void testAliasing() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/aliasing.kt"); + } + @Test public void testAllFilesPresentInVariables() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/variables"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.fir\\.kts?$"), true); diff --git a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirSmartcastProvider.kt b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirSmartcastProvider.kt index 8a507c78ced..254d3b8a5b1 100644 --- a/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirSmartcastProvider.kt +++ b/idea/idea-frontend-fir/src/org/jetbrains/kotlin/idea/frontend/api/fir/components/KtFirSmartcastProvider.kt @@ -9,6 +9,7 @@ import org.jetbrains.kotlin.fir.expressions.FirExpressionWithSmartcast import org.jetbrains.kotlin.fir.expressions.FirQualifiedAccessExpression import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.fir.types.coneTypeSafe +import org.jetbrains.kotlin.fir.types.isStableSmartcast import org.jetbrains.kotlin.idea.fir.low.level.api.api.getOrBuildFirSafe import org.jetbrains.kotlin.idea.frontend.api.ImplicitReceiverSmartCast import org.jetbrains.kotlin.idea.frontend.api.ImplicitReceiverSmartcastKind @@ -41,13 +42,13 @@ internal class KtFirSmartcastProvider( (extensionReceiver !is FirExpressionWithSmartcast || !extensionReceiver.isStable) ) return emptyList() buildList { - (dispatchReceiver as? FirExpressionWithSmartcast)?.takeIf { it.isStable }?.let { smartCasted -> + dispatchReceiver.takeIf { it.isStableSmartcast() }?.let { smartCasted -> ImplicitReceiverSmartCast( smartCasted.typeRef.coneTypeSafe()?.asKtType() ?: return@let null, ImplicitReceiverSmartcastKind.DISPATCH ) }?.let(::add) - (extensionReceiver as? FirExpressionWithSmartcast)?.takeIf { it.isStable }?.let { smartCasted -> + extensionReceiver.takeIf { it.isStableSmartcast() }?.let { smartCasted -> ImplicitReceiverSmartCast( smartCasted.typeRef.coneTypeSafe()?.asKtType() ?: return@let null, ImplicitReceiverSmartcastKind.EXTENSION