From 1936658e4050074a5235479d5c715f2cf41e7a6e Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Fri, 28 Apr 2023 16:37:41 +0300 Subject: [PATCH] [FIR] Consider stability of receiver for DFA variables ^KT-57425 Fixed --- ...CompilerTestFE10TestdataTestGenerated.java | 12 +++ ...sticCompilerFE10TestDataTestGenerated.java | 12 +++ ...eeOldFrontendDiagnosticsTestGenerated.java | 12 +++ ...siOldFrontendDiagnosticsTestGenerated.java | 12 +++ .../kotlin/fir/resolve/dfa/DfaVariables.kt | 24 ++++-- .../publicVals/accessThrowOtherModule.kt | 14 ++++ .../reassignedDependency_unstable.fir.kt | 80 +++++++++++++++++++ .../reassignedDependency_unstable.kt | 80 +++++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 12 +++ 9 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.fir.kt create mode 100644 compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java index ba0ec3b0f53..dc0a42c12c5 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java @@ -30884,6 +30884,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia @TestMetadata("compiler/testData/diagnostics/tests/smartCasts/publicVals") @TestDataPath("$PROJECT_ROOT") public class PublicVals { + @Test + @TestMetadata("accessThrowOtherModule.kt") + public void testAccessThrowOtherModule() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt"); + } + @Test public void testAllFilesPresentInPublicVals() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/publicVals"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform"); @@ -31317,6 +31323,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.kt"); } + @Test + @TestMetadata("reassignedDependency_unstable.kt") + public void testReassignedDependency_unstable() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt"); + } + @Test @TestMetadata("reassignedInRhs.kt") public void testReassignedInRhs() throws Exception { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java index e909a91836b..2303026b2e1 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java @@ -30884,6 +30884,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated @TestMetadata("compiler/testData/diagnostics/tests/smartCasts/publicVals") @TestDataPath("$PROJECT_ROOT") public class PublicVals { + @Test + @TestMetadata("accessThrowOtherModule.kt") + public void testAccessThrowOtherModule() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt"); + } + @Test public void testAllFilesPresentInPublicVals() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/publicVals"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform"); @@ -31317,6 +31323,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.kt"); } + @Test + @TestMetadata("reassignedDependency_unstable.kt") + public void testReassignedDependency_unstable() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt"); + } + @Test @TestMetadata("reassignedInRhs.kt") public void testReassignedInRhs() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index 55de1eec8c0..6bd25e8f1d3 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -30884,6 +30884,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir @TestMetadata("compiler/testData/diagnostics/tests/smartCasts/publicVals") @TestDataPath("$PROJECT_ROOT") public class PublicVals { + @Test + @TestMetadata("accessThrowOtherModule.kt") + public void testAccessThrowOtherModule() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt"); + } + @Test public void testAllFilesPresentInPublicVals() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/publicVals"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform"); @@ -31317,6 +31323,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.kt"); } + @Test + @TestMetadata("reassignedDependency_unstable.kt") + public void testReassignedDependency_unstable() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt"); + } + @Test @TestMetadata("reassignedInRhs.kt") public void testReassignedInRhs() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index 280360ddedb..0d5f251ad8c 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -30980,6 +30980,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia @TestMetadata("compiler/testData/diagnostics/tests/smartCasts/publicVals") @TestDataPath("$PROJECT_ROOT") public class PublicVals { + @Test + @TestMetadata("accessThrowOtherModule.kt") + public void testAccessThrowOtherModule() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt"); + } + @Test public void testAllFilesPresentInPublicVals() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/publicVals"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform"); @@ -31413,6 +31419,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.kt"); } + @Test + @TestMetadata("reassignedDependency_unstable.kt") + public void testReassignedDependency_unstable() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt"); + } + @Test @TestMetadata("reassignedInRhs.kt") public void testReassignedInRhs() throws Exception { diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/DfaVariables.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/DfaVariables.kt index fbabcdad9d1..93669b0abd0 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/DfaVariables.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/DfaVariables.kt @@ -9,7 +9,6 @@ import org.jetbrains.kotlin.fir.FirElement import org.jetbrains.kotlin.fir.expressions.FirResolvedQualifier import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol -import org.jetbrains.kotlin.fir.types.ConeKotlinType import org.jetbrains.kotlin.types.SmartcastStability data class Identifier( @@ -34,6 +33,9 @@ enum class PropertyStability(val impliedSmartcastStability: SmartcastStability?) // Smartcast is definitely safe regardless of usage. STABLE_VALUE(SmartcastStability.STABLE_VALUE), + // Smartcast may or may not be safe, depending on whether there are concurrent writes to this local variable. + LOCAL_VAR(null), + // Open or custom getter. // Smartcast is always unsafe regardless of usage. PROPERTY_WITH_GETTER(SmartcastStability.PROPERTY_WITH_GETTER), @@ -42,16 +44,24 @@ enum class PropertyStability(val impliedSmartcastStability: SmartcastStability?) // Smartcast is always unsafe regardless of usage. ALIEN_PUBLIC_PROPERTY(SmartcastStability.ALIEN_PUBLIC_PROPERTY), - // Smartcast may or may not be safe, depending on whether there are concurrent writes to this local variable. - LOCAL_VAR(null), - // Mutable member property of a class or object. // Smartcast is always unsafe regardless of usage. MUTABLE_PROPERTY(SmartcastStability.MUTABLE_PROPERTY), // Delegated property of a class or object. // Smartcast is always unsafe regardless of usage. - DELEGATED_PROPERTY(SmartcastStability.DELEGATED_PROPERTY), + DELEGATED_PROPERTY(SmartcastStability.DELEGATED_PROPERTY); + + fun combineWithReceiverStability(receiverStability: PropertyStability?): PropertyStability { + if (receiverStability == null) return this + if (this == LOCAL_VAR) { + require(receiverStability == STABLE_VALUE || receiverStability == LOCAL_VAR) { + "LOCAL_VAR can have only stable or local receiver, but got $receiverStability" + } + return this + } + return maxOf(this, receiverStability) + } } class RealVariable( @@ -59,10 +69,12 @@ class RealVariable( val isThisReference: Boolean, val explicitReceiverVariable: DataFlowVariable?, variableIndexForDebug: Int, - val stability: PropertyStability, + stability: PropertyStability, ) : DataFlowVariable(variableIndexForDebug) { val dependentVariables = mutableSetOf() + val stability: PropertyStability = stability.combineWithReceiverStability((explicitReceiverVariable as? RealVariable)?.stability) + override fun equals(other: Any?): Boolean { return this === other } diff --git a/compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt b/compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt new file mode 100644 index 00000000000..b10c36c0413 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt @@ -0,0 +1,14 @@ +// FIR_IDENTICAL +// ISSUE: KT-57425 +// WITH_STDLIB + +data class Data(val x: Int?) + +fun test(pair: Pair) { + if (pair.second.x != null) { + pair.second.x.inc() // should be an error + } + if (pair.first != null) { + pair.first.length // should be an error + } +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.fir.kt new file mode 100644 index 00000000000..76a44d03f6d --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.fir.kt @@ -0,0 +1,80 @@ +// SKIP_TXT + +class C { + val x: String? + get() = null +} + +fun test1() { + var c = C() + val x = c.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + c = C() + x.length // smartcast + c.x.length // no smartcast +} + +fun test2() { + var c = C() + val x = c.x + if (x == null) return + while (true) { + x.length // smartcast + c.x.length // no smartcast + c = C() + x.length // smartcast + c.x.length // no smartcast + } +} + +fun test3(p: Boolean) { + var c = C() + val x = c.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + if (p) { + c = C() + } + x.length // smartcast + c.x.length // no smartcast +} + +fun test4(p: Boolean, q: Boolean) { + var c = C() + val x = c.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + if (p) { + if (q) { + c = C() + } else { + c = C() + } + } else { + if (q) { + c = C() + } else { + c = C() + } + } + x.length // smartcast + c.x.length // no smartcast +} + +fun test5() { + var c = C() + val d = c + val x = d.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + d.x.length // no smartcast + c = C() + x.length // smartcast + c.x.length // no smartcast + d.x.length // no smartcast +} diff --git a/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt b/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt new file mode 100644 index 00000000000..8ea75d76e87 --- /dev/null +++ b/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt @@ -0,0 +1,80 @@ +// SKIP_TXT + +class C { + val x: String? + get() = null +} + +fun test1() { + var c = C() + val x = c.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + c = C() + x.length // smartcast + c.x.length // no smartcast +} + +fun test2() { + var c = C() + val x = c.x + if (x == null) return + while (true) { + x.length // smartcast + c.x.length // no smartcast + c = C() + x.length // smartcast + c.x.length // no smartcast + } +} + +fun test3(p: Boolean) { + var c = C() + val x = c.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + if (p) { + c = C() + } + x.length // smartcast + c.x.length // no smartcast +} + +fun test4(p: Boolean, q: Boolean) { + var c = C() + val x = c.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + if (p) { + if (q) { + c = C() + } else { + c = C() + } + } else { + if (q) { + c = C() + } else { + c = C() + } + } + x.length // smartcast + c.x.length // no smartcast +} + +fun test5() { + var c = C() + val d = c + val x = d.x + if (x == null) return + x.length // smartcast + c.x.length // no smartcast + d.x.length // no smartcast + c = C() + x.length // smartcast + c.x.length // no smartcast + d.x.length // no smartcast +} 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 c4c50b5687e..e9dec78b0a8 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 @@ -31752,6 +31752,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { @TestMetadata("compiler/testData/diagnostics/tests/smartCasts/publicVals") @TestDataPath("$PROJECT_ROOT") public class PublicVals { + @Test + @TestMetadata("accessThrowOtherModule.kt") + public void testAccessThrowOtherModule() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/publicVals/accessThrowOtherModule.kt"); + } + @Test public void testAllFilesPresentInPublicVals() throws Exception { KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/smartCasts/publicVals"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true); @@ -32185,6 +32191,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.kt"); } + @Test + @TestMetadata("reassignedDependency_unstable.kt") + public void testReassignedDependency_unstable() throws Exception { + runTest("compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency_unstable.kt"); + } + @Test @TestMetadata("reassignedInRhs.kt") public void testReassignedInRhs() throws Exception {