From 4bb7b1ac9fc0f8c3f2cdfd9898567ff3a9ae4edb Mon Sep 17 00:00:00 2001 From: pyos Date: Sat, 10 Dec 2022 19:18:15 +0100 Subject: [PATCH] FIR DFA: use class enter node as data flow source for members Also fix graphs for enums with specialized entries - since we don't create property subgraphs for FirEnumEntry, there is no body to insert AnonymousObjectEnterNode, AnonymousObjectExitNode, and AnonymousObjectExpressionExitNode into. --- .../resolve/cfg/annotatedLocalClass.dot | 5 +- .../cfg/innerClassInAnonymousObject.dot | 5 +- .../resolve/cfg/localClassesWithImplicit.dot | 9 ++- .../resolve/cfg/propertiesAndInitBlocks.dot | 10 ++- .../smartcasts/smartcastInByClause.dot | 4 +- .../bad/callsInPlace/inAnonymousObject.dot | 4 +- .../bad/callsInPlace/inLocalClass.dot | 5 +- .../delegates/delegateWithAnonymousObject.dot | 4 +- .../fir/resolve/dfa/FirDataFlowAnalyzer.kt | 6 +- .../dfa/cfg/ControlFlowGraphBuilder.kt | 72 ++++++++----------- .../body/resolve/FirImplicitBodyResolve.kt | 9 +-- .../body/resolve/LocalClassesNavigation.kt | 9 +-- 12 files changed, 56 insertions(+), 86 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/annotatedLocalClass.dot b/compiler/fir/analysis-tests/testData/resolve/cfg/annotatedLocalClass.dot index 7937b68d580..ab7fb2beece 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/annotatedLocalClass.dot +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/annotatedLocalClass.dot @@ -79,13 +79,12 @@ digraph annotatedLocalClass_kt { 16 -> {17} [style=dotted]; 17 -> {18} [style=dotted]; 18 -> {19}; - 18 -> {25} [color=red]; - 19 -> {20}; - 19 -> {23} [color=green]; + 19 -> {20 23}; 19 -> {23} [style=dashed]; 20 -> {21}; 21 -> {22}; 23 -> {24} [color=green]; + 23 -> {25} [color=red]; 24 -> {25} [color=green]; 24 -> {25} [style=dashed]; 25 -> {26}; diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.dot b/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.dot index f73ad1d41ca..877bff81b19 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.dot +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/innerClassInAnonymousObject.dot @@ -44,18 +44,19 @@ digraph innerClassInAnonymousObject_kt { 2 [label="Exit function " style="filled" fillcolor=red]; } 14 -> {15}; - 14 -> {0 3 6} [color=red]; - 15 -> {12} [color=green]; + 15 -> {12}; 15 -> {16} [color=red]; 15 -> {12} [style=dashed]; 16 -> {17}; 17 -> {18}; 12 -> {13} [color=green]; + 12 -> {0 10} [color=red]; 13 -> {0 10 16} [color=green]; 13 -> {0 10} [style=dashed]; 0 -> {1}; 1 -> {2}; 10 -> {11} [color=green]; + 10 -> {3 6} [color=red]; 11 -> {3 6} [color=green]; 11 -> {3 6} [style=dashed]; 3 -> {4}; diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot index 88c0389b64c..d5f8c0394ec 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot @@ -351,12 +351,9 @@ digraph localClassesWithImplicit_kt { 19 -> {20} [style=dotted]; 20 -> {21} [style=dotted]; 21 -> {22}; - 21 -> {31 34 71 90} [color=red]; - 22 -> {23}; - 22 -> {29} [color=green]; - 22 -> {99 102 139 158} [color=red]; + 22 -> {23 29}; 22 -> {29} [style=dashed]; - 23 -> {97} [color=green]; + 23 -> {97}; 23 -> {24} [color=red]; 23 -> {97} [style=dashed]; 24 -> {25}; @@ -364,6 +361,7 @@ digraph localClassesWithImplicit_kt { 26 -> {27}; 27 -> {28}; 29 -> {30} [color=green]; + 29 -> {31 34 71 90} [color=red]; 30 -> {31 34 71 90} [color=green]; 30 -> {31 34 71 90} [style=dashed]; 31 -> {32}; @@ -436,6 +434,7 @@ digraph localClassesWithImplicit_kt { 94 -> {95} [style=dotted]; 95 -> {96} [style=dotted]; 97 -> {98} [color=green]; + 97 -> {99 102 139 158} [color=red]; 98 -> {24 99 102 139 158} [color=green]; 98 -> {99 102 139 158} [style=dashed]; 99 -> {100}; diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/propertiesAndInitBlocks.dot b/compiler/fir/analysis-tests/testData/resolve/cfg/propertiesAndInitBlocks.dot index ce422dfb852..cdb6309465d 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/propertiesAndInitBlocks.dot +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/propertiesAndInitBlocks.dot @@ -114,12 +114,11 @@ digraph propertiesAndInitBlocks_kt { } 58 -> {59}; 59 -> {60}; - 59 -> {66 73} [color=red]; - 60 -> {61}; - 60 -> {63} [color=green]; + 60 -> {61 63}; 60 -> {63} [style=dashed]; 61 -> {62}; 63 -> {64} [color=green]; + 63 -> {66 73} [color=red]; 64 -> {66} [color=green]; 64 -> {65} [style=dotted]; 64 -> {66} [style=dashed]; @@ -210,10 +209,8 @@ digraph propertiesAndInitBlocks_kt { 24 -> {25}; 25 -> {26}; 26 -> {27 33}; - 26 -> {47 55} [color=red]; 26 -> {33} [style=dashed]; - 27 -> {28}; - 27 -> {44} [color=green]; + 27 -> {28 44}; 27 -> {44} [style=dashed]; 28 -> {29}; 29 -> {30} [style=dotted]; @@ -231,6 +228,7 @@ digraph propertiesAndInitBlocks_kt { 41 -> {42} [style=dotted]; 42 -> {43} [style=dotted]; 44 -> {45} [color=green]; + 44 -> {47 55} [color=red]; 45 -> {47} [color=green]; 45 -> {46} [style=dotted]; 45 -> {47} [style=dashed]; diff --git a/compiler/fir/analysis-tests/testData/resolve/smartcasts/smartcastInByClause.dot b/compiler/fir/analysis-tests/testData/resolve/smartcasts/smartcastInByClause.dot index 1adeb27688c..dc44604c45c 100644 --- a/compiler/fir/analysis-tests/testData/resolve/smartcasts/smartcastInByClause.dot +++ b/compiler/fir/analysis-tests/testData/resolve/smartcasts/smartcastInByClause.dot @@ -192,8 +192,7 @@ digraph smartcastInByClause_kt { 39 -> {40}; 40 -> {41}; 41 -> {42}; - 41 -> {53 59 64 67} [color=red]; - 42 -> {49} [color=green]; + 42 -> {49}; 42 -> {43} [color=red]; 42 -> {49} [style=dashed]; 43 -> {44}; @@ -203,6 +202,7 @@ digraph smartcastInByClause_kt { 46 -> {47} [style=dotted]; 47 -> {48} [style=dotted]; 49 -> {50} [color=green]; + 49 -> {53 59 64 67} [color=red]; 50 -> {53} [color=green]; 50 -> {51} [style=dotted]; 50 -> {53} [style=dashed]; diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inAnonymousObject.dot b/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inAnonymousObject.dot index d05f3daf0f8..e2a5a8db021 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inAnonymousObject.dot +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inAnonymousObject.dot @@ -71,8 +71,7 @@ digraph inAnonymousObject_kt { 1 -> {2}; 2 -> {3}; 3 -> {4}; - 3 -> {17 20 26 29} [color=red]; - 4 -> {13} [color=green]; + 4 -> {13}; 4 -> {5} [color=red]; 4 -> {13} [style=dashed]; 5 -> {6}; @@ -83,6 +82,7 @@ digraph inAnonymousObject_kt { 10 -> {11}; 11 -> {12}; 13 -> {14} [color=green]; + 13 -> {17 20 26 29} [color=red]; 14 -> {17} [color=green]; 14 -> {15} [style=dotted]; 14 -> {17} [style=dashed]; diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inLocalClass.dot b/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inLocalClass.dot index 4d560127dbf..ab2319a7e57 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inLocalClass.dot +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/contracts/fromSource/bad/callsInPlace/inLocalClass.dot @@ -74,15 +74,14 @@ digraph inLocalClass_kt { 1 -> {2}; 2 -> {3}; 3 -> {4}; - 3 -> {14 17 23 29} [color=red]; - 4 -> {5}; - 4 -> {10} [color=green]; + 4 -> {5 10}; 4 -> {10} [style=dashed]; 5 -> {6}; 6 -> {7}; 7 -> {8}; 8 -> {9}; 10 -> {11} [color=green]; + 10 -> {14 17 23 29} [color=red]; 11 -> {14} [color=green]; 11 -> {12} [style=dotted]; 11 -> {14} [style=dashed]; diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot b/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot index 6a3b54d7316..6b4c9ee9127 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot @@ -196,14 +196,14 @@ digraph delegateWithAnonymousObject_kt { 78 -> {81} [color=green]; 26 -> {27}; 27 -> {28}; - 27 -> {35 38 45} [color=red]; - 28 -> {33} [color=green]; + 28 -> {33}; 28 -> {29} [color=red]; 28 -> {33} [style=dashed]; 29 -> {30}; 30 -> {31}; 31 -> {32}; 33 -> {34} [color=green]; + 33 -> {35 38 45} [color=red]; 34 -> {29 35 38 45} [color=green]; 34 -> {35 38 45} [style=dashed]; 35 -> {36}; diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt index c833871563d..cccdb88f60d 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt @@ -204,7 +204,9 @@ abstract class FirDataFlowAnalyzer( // ----------------------------------- Classes ----------------------------------- fun enterClass(klass: FirClass, buildGraph: Boolean) { - graphBuilder.enterClass(klass, buildGraph)?.mergeIncomingFlow() + val (outerNode, enterNode) = graphBuilder.enterClass(klass, buildGraph) ?: return + outerNode?.mergeIncomingFlow() + enterNode.mergeIncomingFlow() } fun exitClass(): ControlFlowGraph? { @@ -223,7 +225,7 @@ abstract class FirDataFlowAnalyzer( } fun exitAnonymousObjectExpression(anonymousObjectExpression: FirAnonymousObjectExpression) { - graphBuilder.exitAnonymousObjectExpression(anonymousObjectExpression).mergeIncomingFlow() + graphBuilder.exitAnonymousObjectExpression(anonymousObjectExpression)?.mergeIncomingFlow() } // ----------------------------------- Scripts ------------------------------------------ diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt index b6a457dd9b0..c65071d816d 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.fir.resolve.dfa.cfg import org.jetbrains.kotlin.contracts.description.* +import org.jetbrains.kotlin.descriptors.ClassKind import org.jetbrains.kotlin.fir.FirElement import org.jetbrains.kotlin.fir.declarations.* import org.jetbrains.kotlin.fir.declarations.utils.hasExplicitBackingField @@ -43,7 +44,7 @@ class ControlFlowGraphBuilder { private val exitTargetsForReturn: SymbolBasedNodeStorage = SymbolBasedNodeStorage() private val exitTargetsForTry: Stack> = stackOf() - private val enterToLocalClassesMembers: MutableMap, CFGNode<*>?> = mutableMapOf() + private val enterToLocalClassesMembers: MutableMap, CFGNode<*>> = mutableMapOf() //return jumps via finally blocks, target -> jumps private val nonDirectJumps: ListMultimap, CFGNode<*>> = listMultimapOf() @@ -361,19 +362,20 @@ class ControlFlowGraphBuilder { // ----------------------------------- Classes ----------------------------------- - fun enterClass(klass: FirClass, buildGraph: Boolean): CFGNode<*>? { + fun enterClass(klass: FirClass, buildGraph: Boolean): Pair?, ClassEnterNode>? { if (!buildGraph) { pushGraph(ControlFlowGraph(null, "STUB_CLASS_GRAPH", ControlFlowGraph.Kind.Stub)) return null } val enterNode = when { - klass is FirAnonymousObject -> createAnonymousObjectEnterNode(klass) + // TODO: enum classes cannot be local so this is mostly fine, but it looks hacky. Maybe handle FirEnumEntry? + klass is FirAnonymousObject && klass.classKind != ClassKind.ENUM_ENTRY -> createAnonymousObjectEnterNode(klass) // Local classes are only initialized on first use, so they look pretty much like named functions: // control flow enters here and never leaves, and assignments invalidate smart casts. klass is FirRegularClass && klass.isLocal && currentGraph.kind.withBody -> createLocalClassExitNode(klass) else -> null - } + }?.also { addNewSimpleNode(it) } val name = when (klass) { is FirAnonymousObject -> "" @@ -383,14 +385,25 @@ class ControlFlowGraphBuilder { pushGraph(ControlFlowGraph(klass, name, ControlFlowGraph.Kind.ClassInitializer)) val graphEnterNode = createClassEnterNode(klass) + lastNodes.push(graphEnterNode) if (enterNode != null) { - // TODO: anonymous objects are used to represent enum entries - check what happens there - // (likely `exitClass` leaves a node on the stack that will never be consumed) - lastNodes.popOrNull()?.let { addEdge(it, enterNode) } - lastNodes.push(enterNode) - addEdge(enterNode, graphEnterNode, preferredKind = EdgeKind.CfgForward) + addEdge(enterNode, graphEnterNode) + } else { + enterToLocalClassesMembers.remove(klass.symbol)?.let { addEdge(it, graphEnterNode, preferredKind = EdgeKind.DfgForward) } } - return enterNode + + if (graphEnterNode.previousNodes.isNotEmpty()) { + for (member in klass.declarations) { + if (member is FirFunction || member is FirAnonymousInitializer || member is FirField || member is FirClass || member is FirProperty) { + enterToLocalClassesMembers[member.symbol] = graphEnterNode + } + if (member is FirProperty) { + member.getter?.let { enterToLocalClassesMembers[it.symbol] = graphEnterNode } + member.setter?.let { enterToLocalClassesMembers[it.symbol] = graphEnterNode } + } + } + } + return enterNode to graphEnterNode } fun exitClass(): Pair? { @@ -400,13 +413,14 @@ class ControlFlowGraphBuilder { } val graph = popClassGraph() - if (graph.declaration !is FirAnonymousObject) { + val anonymousObject = graph.declaration as? FirAnonymousObject + if (anonymousObject == null || anonymousObject.classKind == ClassKind.ENUM_ENTRY) { return null to graph } val lastNode = lastNodes.pop() as AnonymousObjectEnterNode val exitNode = createAnonymousObjectExitNode(lastNode.fir).also { lastNodes.push(it) } - // TODO: should merge data flow from members into the exit node. + // TODO: should merge data flow from initializer parts into the exit node. // TODO: the reason for this liveness trickery is that if control flow is dead, the CFG-only edge from // `graph.exitNode` gets magically transformed into a CFG+DFG dead edge, and `graph.exitNode` has no data // flow information attached to it. This should be fixed as soon as data flow is made to go through the object. @@ -439,7 +453,7 @@ class ControlFlowGraphBuilder { } } - var node: CFGNode<*> = currentGraph.enterNode as ClassEnterNode + var node: CFGNode<*> = lastNodes.pop() as ClassEnterNode var prevInitPartNode: CFGNode<*>? = null for (graph in calledInPlace) { val partNode = createPartOfClassInitializationNode(graph.declaration as FirControlFlowGraphOwner) @@ -476,22 +490,9 @@ class ControlFlowGraphBuilder { return popGraph() } - fun prepareForLocalClassMembers(members: Collection) { - // TODO: this is called before `enterClass` so the data flow source for objects and local classes - // is not the enter node, but whichever node happens to be before it. This technically works, - // but is ugly. - members.forEachMember { - enterToLocalClassesMembers[it.symbol] = lastNodes.topOrNull() - } - } + fun exitAnonymousObjectExpression(anonymousObjectExpression: FirAnonymousObjectExpression): AnonymousObjectExpressionExitNode? { + if (anonymousObjectExpression.anonymousObject.classKind == ClassKind.ENUM_ENTRY) return null - fun cleanAfterForLocalClassMembers(members: Collection) { - members.forEachMember { - enterToLocalClassesMembers.remove(it.symbol) - } - } - - fun exitAnonymousObjectExpression(anonymousObjectExpression: FirAnonymousObjectExpression): AnonymousObjectExpressionExitNode { // TODO: what's AnonymousObjectExitNode for then? return createAnonymousObjectExpressionExitNode(anonymousObjectExpression).also { addNewSimpleNodeIfPossible(it) @@ -1402,21 +1403,6 @@ class ControlFlowGraphBuilder { // ----------------------------------- Utils ----------------------------------- - private inline fun Collection.forEachMember(block: (FirDeclaration) -> Unit) { - for (member in this) { - for (callableDeclaration in member.unwrap()) { - block(callableDeclaration) - } - } - } - - private fun FirDeclaration.unwrap(): List = - when (this) { - is FirFunction, is FirAnonymousInitializer, is FirField -> listOf(this) - is FirProperty -> listOfNotNull(this.getter, this.setter, this) - else -> emptyList() - } - private fun withLevelOfNode(node: CFGNode<*>, f: () -> R): R { val last = levelCounter levelCounter = node.level diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt index a0cc06237f9..14ee6774251 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt @@ -102,14 +102,7 @@ fun F.runContractAndBodiesResolutionForLocalClass( outerBodyResolveContext = newContext, firTowerDataContextCollector = firTowerDataContextCollector ) - - val graphBuilder = components.context.dataFlowAnalyzerContext.graphBuilder - val members = localClassesNavigationInfo.allMembers - graphBuilder.prepareForLocalClassMembers(members) - - return this.transform(transformer, resolutionMode).also { - graphBuilder.cleanAfterForLocalClassMembers(members) - } + return this.transform(transformer, resolutionMode) } private fun ReturnTypeCalculator.getTransformerCreator() = when (this) { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/LocalClassesNavigation.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/LocalClassesNavigation.kt index 8f2e7e9bc30..133927dd7df 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/LocalClassesNavigation.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/LocalClassesNavigation.kt @@ -14,7 +14,6 @@ import org.jetbrains.kotlin.utils.keysToMap class LocalClassesNavigationInfo( val parentForClass: Map, private val parentClassForFunction: Map, - val allMembers: List ) { val designationMap: Map> by lazy { parentClassForFunction.keys.keysToMap { @@ -39,13 +38,12 @@ fun FirClassLikeDeclaration.collectLocalClassesNavigationInfo(): LocalClassesNav NavigationInfoVisitor().run { this@collectLocalClassesNavigationInfo.accept(this@run, null) - LocalClassesNavigationInfo(parentForClass, resultingMap, allMembers) + LocalClassesNavigationInfo(parentForClass, resultingMap) } private class NavigationInfoVisitor : FirDefaultVisitor() { val resultingMap: MutableMap = mutableMapOf() val parentForClass: MutableMap = mutableMapOf() - val allMembers: MutableList = mutableListOf() private val currentPath: MutableList = mutableListOf() override fun visitElement(element: FirElement, data: Any?) {} @@ -84,12 +82,7 @@ private class NavigationInfoVisitor : FirDefaultVisitor() { } override fun visitCallableDeclaration(callableDeclaration: FirCallableDeclaration, data: Any?) { - allMembers += callableDeclaration if (callableDeclaration.returnTypeRef !is FirImplicitTypeRef) return resultingMap[callableDeclaration] = currentPath.last() } - - override fun visitAnonymousInitializer(anonymousInitializer: FirAnonymousInitializer, data: Any?) { - allMembers += anonymousInitializer - } }