Skip to content

Commit 8943013

Browse files
bnormSpace Team
authored and
Space Team
committed
[FIR] Merge loop and lambda assignment look-ahead into single class
There are two classes that essentially do the same thing: look-ahead at future assignments to determine smartcast stability. One class is used primarily for loops (PreliminaryLoopVisitor), and the other is used primarily for lambdas (FirLocalVariableAssignmentAnalyzer). Merge their behavior to avoid smartcast inconsistencies between loops and lambdas. ^KT-57678 Fixed ^KT-59688 Fixed
1 parent 73cb9f4 commit 8943013

File tree

4 files changed

+76
-143
lines changed

4 files changed

+76
-143
lines changed

compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ import org.jetbrains.kotlin.util.OperatorNameConventions
4444

4545
class DataFlowAnalyzerContext(session: FirSession) {
4646
val graphBuilder = ControlFlowGraphBuilder()
47-
val preliminaryLoopVisitor = PreliminaryLoopVisitor()
48-
val variablesClearedBeforeLoop = stackOf<List<RealVariable>>()
4947
internal val variableAssignmentAnalyzer = FirLocalVariableAssignmentAnalyzer()
5048

5149
var variableStorage = VariableStorageImpl(session)
@@ -59,8 +57,6 @@ class DataFlowAnalyzerContext(session: FirSession) {
5957

6058
fun reset() {
6159
graphBuilder.reset()
62-
preliminaryLoopVisitor.resetState()
63-
variablesClearedBeforeLoop.reset()
6460
variableAssignmentAnalyzer.reset()
6561
variableStorage = variableStorage.clear()
6662
}
@@ -181,6 +177,8 @@ abstract class FirDataFlowAnalyzer(
181177
fun enterFunction(function: FirFunction) {
182178
if (function is FirDefaultPropertyAccessor) return
183179

180+
val assignedInside = context.variableAssignmentAnalyzer.enterFunction(function)
181+
184182
val (localFunctionNode, functionEnterNode) = if (function is FirAnonymousFunction) {
185183
null to graphBuilder.enterAnonymousFunction(function)
186184
} else {
@@ -192,22 +190,17 @@ abstract class FirDataFlowAnalyzer(
192190
* Anonymous functions which can be revisited, either in-place or not in-place, are treated as repeatable statements. This
193191
* causes any assignments to local variables within the anonymous function body to clear type statements for those local
194192
* variables.
195-
* TODO(KT-57678): is it possible for FirLocalVariableAssignmentAnalyzer to handle this for both lambdas and loops?
196193
*/
197194
if (function is FirAnonymousFunction && function.invocationKind?.canBeRevisited() != false) {
198-
enterRepeatableStatement(flow, function)
195+
enterRepeatableStatement(flow, assignedInside)
199196
}
200197
}
201-
context.variableAssignmentAnalyzer.enterFunction(function)
202198
}
203199

204200
fun exitFunction(function: FirFunction): FirControlFlowGraphReference? {
205201
if (function is FirDefaultPropertyAccessor) return null
206202

207203
context.variableAssignmentAnalyzer.exitFunction()
208-
if (function is FirAnonymousFunction && function.invocationKind?.canBeRevisited() != false) {
209-
exitRepeatableStatement(function)
210-
}
211204

212205
if (function is FirAnonymousFunction) {
213206
val (functionExitNode, postponedLambdaExitNode, graph) = graphBuilder.exitAnonymousFunction(function)
@@ -699,9 +692,10 @@ abstract class FirDataFlowAnalyzer(
699692
// ----------------------------------- While Loop -----------------------------------
700693

701694
fun enterWhileLoop(loop: FirLoop) {
695+
val assignedInside = context.variableAssignmentAnalyzer.enterLoop(loop)
702696
val (loopEnterNode, loopConditionEnterNode) = graphBuilder.enterWhileLoop(loop)
703697
loopEnterNode.mergeIncomingFlow()
704-
loopConditionEnterNode.mergeIncomingFlow { _, flow -> enterRepeatableStatement(flow, loop) }
698+
loopConditionEnterNode.mergeIncomingFlow { _, flow -> enterRepeatableStatement(flow, assignedInside) }
705699
}
706700

707701
fun exitWhileLoopCondition(loop: FirLoop) {
@@ -716,17 +710,25 @@ abstract class FirDataFlowAnalyzer(
716710
}
717711

718712
fun exitWhileLoop(loop: FirLoop) {
713+
val assignedInside = context.variableAssignmentAnalyzer.exitLoop()
719714
val (conditionEnterNode, blockExitNode, exitNode) = graphBuilder.exitWhileLoop(loop)
720715
blockExitNode.mergeIncomingFlow()
721716
exitNode.mergeIncomingFlow { path, flow ->
722-
processWhileLoopExit(path, flow, exitNode, conditionEnterNode)
717+
processWhileLoopExit(path, flow, exitNode, conditionEnterNode, assignedInside)
723718
processLoopExit(flow, exitNode, exitNode.firstPreviousNode as LoopConditionExitNode)
724719
}
725720
}
726721

727-
private fun processWhileLoopExit(path: FlowPath, flow: MutableFlow, node: LoopExitNode, conditionEnterNode: LoopConditionEnterNode) {
728-
val possiblyChangedVariables = exitRepeatableStatement(node.fir)
729-
if (possiblyChangedVariables.isNullOrEmpty()) return
722+
private fun processWhileLoopExit(
723+
path: FlowPath,
724+
flow: MutableFlow,
725+
node: LoopExitNode,
726+
conditionEnterNode: LoopConditionEnterNode,
727+
reassigned: Set<FirPropertySymbol>,
728+
) {
729+
if (reassigned.isEmpty()) return
730+
val possiblyChangedVariables = variableStorage.realVariables.values.filter { it.identifier.symbol in reassigned }
731+
if (possiblyChangedVariables.isEmpty()) return
730732
// While analyzing the loop we might have added some backwards jumps to `conditionEnterNode` which weren't
731733
// there at the time its flow was computed - which is why we erased all information about `possiblyChangedVariables`
732734
// from it. Now that we have those edges, we can restore type information for the code after the loop.
@@ -758,33 +760,20 @@ abstract class FirDataFlowAnalyzer(
758760
}
759761
}
760762

761-
private fun enterRepeatableStatement(flow: MutableFlow, statement: FirStatement) {
762-
val reassignedNames = context.preliminaryLoopVisitor.enterCapturingStatement(statement)
763-
if (reassignedNames.isEmpty()) return
764-
// TODO: only choose the innermost variable for each name, KT-59688
765-
val possiblyChangedVariables = variableStorage.realVariables.values.filter {
766-
val identifier = it.identifier
767-
val symbol = identifier.symbol
768-
// Non-local vars can never produce stable smart casts anyway.
769-
identifier.dispatchReceiver == null && identifier.extensionReceiver == null &&
770-
symbol is FirPropertySymbol && symbol.isVar && symbol.name in reassignedNames
771-
}
763+
private fun enterRepeatableStatement(flow: MutableFlow, reassigned: Set<FirPropertySymbol>) {
764+
if (reassigned.isEmpty()) return
765+
val possiblyChangedVariables = variableStorage.realVariables.values.filter { it.identifier.symbol in reassigned }
772766
for (variable in possiblyChangedVariables) {
773767
logicSystem.recordNewAssignment(flow, variable, context.newAssignmentIndex())
774768
}
775-
context.variablesClearedBeforeLoop.push(possiblyChangedVariables)
776-
}
777-
778-
private fun exitRepeatableStatement(statement: FirStatement): List<RealVariable>? {
779-
if (context.preliminaryLoopVisitor.exitCapturingStatement(statement).isEmpty()) return null
780-
return context.variablesClearedBeforeLoop.pop()
781769
}
782770

783771
// ----------------------------------- Do while Loop -----------------------------------
784772

785773
fun enterDoWhileLoop(loop: FirLoop) {
774+
val assignedInside = context.variableAssignmentAnalyzer.enterLoop(loop)
786775
val (loopEnterNode, loopBlockEnterNode) = graphBuilder.enterDoWhileLoop(loop)
787-
loopEnterNode.mergeIncomingFlow { _, flow -> enterRepeatableStatement(flow, loop) }
776+
loopEnterNode.mergeIncomingFlow { _, flow -> enterRepeatableStatement(flow, assignedInside) }
788777
loopBlockEnterNode.mergeIncomingFlow()
789778
}
790779

@@ -795,12 +784,12 @@ abstract class FirDataFlowAnalyzer(
795784
}
796785

797786
fun exitDoWhileLoop(loop: FirLoop) {
787+
context.variableAssignmentAnalyzer.exitLoop()
798788
val (loopConditionExitNode, loopExitNode) = graphBuilder.exitDoWhileLoop(loop)
799789
loopConditionExitNode.mergeIncomingFlow()
800790
loopExitNode.mergeIncomingFlow { _, flow ->
801791
processLoopExit(flow, loopExitNode, loopConditionExitNode)
802792
}
803-
exitRepeatableStatement(loop)
804793
}
805794

806795
// ----------------------------------- Try-catch-finally -----------------------------------

compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirLocalVariableAssignmentAnalyzer.kt

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,18 @@
55

66
package org.jetbrains.kotlin.fir.resolve.dfa
77

8+
import org.jetbrains.kotlin.KtFakeSourceElementKind
89
import org.jetbrains.kotlin.contracts.description.isInPlace
910
import org.jetbrains.kotlin.fir.FirElement
1011
import org.jetbrains.kotlin.fir.FirSession
1112
import org.jetbrains.kotlin.fir.declarations.*
1213
import org.jetbrains.kotlin.fir.expressions.*
13-
import org.jetbrains.kotlin.fir.expressions.explicitReceiver
1414
import org.jetbrains.kotlin.fir.references.FirNamedReference
1515
import org.jetbrains.kotlin.fir.references.FirReference
1616
import org.jetbrains.kotlin.fir.references.toResolvedPropertySymbol
1717
import org.jetbrains.kotlin.fir.symbols.FirBasedSymbol
1818
import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol
19+
import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
1920
import org.jetbrains.kotlin.fir.types.ConeKotlinType
2021
import org.jetbrains.kotlin.fir.types.typeContext
2122
import org.jetbrains.kotlin.fir.visitors.FirVisitor
@@ -31,7 +32,7 @@ import org.jetbrains.kotlin.types.AbstractTypeChecker
3132
**/
3233
internal class FirLocalVariableAssignmentAnalyzer {
3334
private var rootFunction: FirFunctionSymbol<*>? = null
34-
private var assignedLocalVariablesByDeclaration: Map<FirBasedSymbol<*>, Fork>? = null
35+
private var assignedLocalVariablesByDeclaration: Map<Any /* FirBasedSymbol<*> | FirLoop */, Fork>? = null
3536
private var variableAssignments: Map<FirProperty, List<Assignment>>? = null
3637

3738
private val scopes: Stack<Pair<Fork?, VariableAssignments>> = stackOf()
@@ -82,14 +83,14 @@ internal class FirLocalVariableAssignmentAnalyzer {
8283
return assignments.all { AbstractTypeChecker.isSubtypeOf(session.typeContext, it.type!!, targetType) }
8384
}
8485

85-
private fun getInfoForDeclaration(symbol: FirBasedSymbol<*>): Fork? {
86+
private fun getInfoForDeclaration(symbol: Any): Fork? {
8687
val root = rootFunction ?: return null
8788
if (root == symbol) return null
8889
val cachedMap = buildInfoForRoot(root)
8990
return cachedMap[symbol]
9091
}
9192

92-
private fun buildInfoForRoot(root: FirFunctionSymbol<*>): Map<FirBasedSymbol<*>, Fork> {
93+
private fun buildInfoForRoot(root: FirFunctionSymbol<*>): Map<Any, Fork> {
9394
assignedLocalVariablesByDeclaration?.let { return it }
9495

9596
val data = MiniCfgBuilder.MiniCfgData()
@@ -102,7 +103,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
102103
}
103104

104105
private fun enterScope(
105-
symbol: FirBasedSymbol<*>,
106+
symbol: Any,
106107
evaluatedInPlace: Boolean,
107108
): Pair<Fork?, VariableAssignments> {
108109
val currentInfo = getInfoForDeclaration(symbol)
@@ -134,11 +135,15 @@ internal class FirLocalVariableAssignmentAnalyzer {
134135
return scopes.top()
135136
}
136137

137-
fun enterFunction(function: FirFunction) {
138+
/**
139+
* Enters an [FirFunction] and returns all [FirPropertySymbol]s which are defined before the function that will be modified within the
140+
* function.
141+
*/
142+
fun enterFunction(function: FirFunction): Set<FirPropertySymbol> {
138143
if (rootFunction == null) {
139144
rootFunction = function.symbol
140145
scopes.push(null to VariableAssignments())
141-
return
146+
return emptySet()
142147
}
143148
val (info, prohibitSmartCasts) =
144149
enterScope(function.symbol, function is FirAnonymousFunction && function.invocationKind.isInPlace)
@@ -149,6 +154,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
149154
}
150155
}
151156
}
157+
return scopes.top().first?.assignedInside?.getAssignedProperties().orEmpty()
152158
}
153159

154160
fun exitFunction() {
@@ -202,6 +208,24 @@ internal class FirLocalVariableAssignmentAnalyzer {
202208
}
203209
}
204210

211+
/**
212+
* Enters an [FirLoop] and returns all [FirPropertySymbol]s which are defined before the loop that will be modified within the loop.
213+
*/
214+
fun enterLoop(loop: FirLoop): Set<FirPropertySymbol> {
215+
if (rootFunction == null) return emptySet()
216+
val (info, _) = enterScope(loop, evaluatedInPlace = true)
217+
return info?.assignedInside?.getAssignedProperties().orEmpty()
218+
}
219+
220+
/**
221+
* Exits an [FirLoop] and returns all [FirPropertySymbol]s which were defined before the loop that were modified within the loop.
222+
*/
223+
fun exitLoop(): Set<FirPropertySymbol> {
224+
if (rootFunction == null) return emptySet()
225+
val (info, _) = scopes.pop()
226+
return info?.assignedInside?.getAssignedProperties().orEmpty()
227+
}
228+
205229
fun visitAssignment(property: FirProperty, type: ConeKotlinType) {
206230
buildInfoForRoot(rootFunction ?: return)
207231
val assignments = variableAssignments?.get(property) ?: return
@@ -293,6 +317,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
293317
)
294318

295319
private class Assignment(
320+
val operatorAssignment: Boolean,
296321
var type: ConeKotlinType? = null,
297322
)
298323

@@ -329,6 +354,13 @@ internal class FirLocalVariableAssignmentAnalyzer {
329354
}
330355

331356
fun isEmpty(): Boolean = assignments.isEmpty()
357+
358+
fun getAssignedProperties(): Set<FirPropertySymbol> {
359+
return assignments.entries
360+
// TODO(KT-57563): Operator assignments should be treated just like any other assignment.
361+
.filter { (_, v) -> v.any { !it.operatorAssignment } }
362+
.mapTo(mutableSetOf()) { (k, _) -> k.symbol }
363+
}
332364
}
333365

334366
private class MiniFlow(val parents: Set<MiniFlow>) {
@@ -412,8 +444,8 @@ internal class FirLocalVariableAssignmentAnalyzer {
412444
// All forks in the loop should have the same set of variables assigned later, equal to the set
413445
// at the start of the loop.
414446
data.flow.recordAssignments(assignedInside)
415-
// The loop flows are detached from the entry flow, so we need to re-join them.
416-
data.flow = setOf(entry, data.flow).join()
447+
data.flow = entry.fork()
448+
data.forks[loop] = Fork(data.flow.assignedLater, assignedInside)
417449
}
418450

419451
override fun visitWhileLoop(whileLoop: FirWhileLoop, data: MiniCfgData) =
@@ -457,21 +489,24 @@ internal class FirLocalVariableAssignmentAnalyzer {
457489
override fun visitVariableAssignment(variableAssignment: FirVariableAssignment, data: MiniCfgData) {
458490
visitElement(variableAssignment, data)
459491
if (variableAssignment.explicitReceiver != null) return
460-
variableAssignment.calleeReference?.let { data.recordAssignment(it) }
492+
variableAssignment.calleeReference?.let {
493+
val operatorAssignment = variableAssignment.source?.kind is KtFakeSourceElementKind.DesugaredIncrementOrDecrement
494+
data.recordAssignment(it, operatorAssignment)
495+
}
461496
}
462497

463498
override fun visitAssignmentOperatorStatement(assignmentOperatorStatement: FirAssignmentOperatorStatement, data: MiniCfgData) {
464499
visitElement(assignmentOperatorStatement, data)
465500
val lhs = assignmentOperatorStatement.leftArgument as? FirQualifiedAccessExpression ?: return
466501
if (lhs.explicitReceiver != null) return
467-
data.recordAssignment(lhs.calleeReference)
502+
data.recordAssignment(lhs.calleeReference, operatorAssignment = true)
468503
}
469504

470-
private fun MiniCfgData.recordAssignment(reference: FirReference) {
505+
private fun MiniCfgData.recordAssignment(reference: FirReference, operatorAssignment: Boolean) {
471506
val name = (reference as? FirNamedReference)?.name ?: return
472507
val property = variableDeclarations.lastOrNull { name in it }?.get(name) ?: return
473508

474-
val assignment = Assignment()
509+
val assignment = Assignment(operatorAssignment)
475510
assignments.getOrPut(property) { mutableListOf() }.add(assignment)
476511
flow.recordAssignment(property, assignment)
477512
}
@@ -492,7 +527,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
492527
var flow: MiniFlow = MiniFlow.start()
493528
val variableDeclarations: ArrayDeque<MutableMap<Name, FirProperty>> = ArrayDeque(listOf(mutableMapOf()))
494529
val assignments: MutableMap<FirProperty, MutableList<Assignment>> = mutableMapOf()
495-
val forks: MutableMap<FirBasedSymbol<*>, Fork> = mutableMapOf()
530+
val forks: MutableMap<Any /* FirBasedSymbol<*> | FirLoop */, Fork> = mutableMapOf()
496531
}
497532
}
498533
}

0 commit comments

Comments
 (0)