azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FIR checker: make property init analyzer path-sensitive

Browse Source 
In particular, exception throwing path after finally block is
distinguished via path label.

KT-42350 Fixed
This commit is contained in:
Jinseong Jeon
2020-10-09 00:12:44 -07:00
committed by Dmitriy Novozhilov
parent 6fc3f7e776
commit ed188204b4
8 changed files with 199 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/AbstractFirPropertyInitializationChecker.kt
+1 -1
View File
@@ -14,7 +14,7 @@ abstract class AbstractFirPropertyInitializationChecker {
abstract fun analyze(
graph: ControlFlowGraph,
reporter: DiagnosticReporter,
data: Map<CFGNode<*>, PropertyInitializationInfo>,
data: Map<CFGNode<*>, PathAwarePropertyInitializationInfo>,
properties: Set<FirPropertySymbol>
)
}
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfaUtils.kt
+105 -16
View File
@@ -11,7 +11,7 @@ import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange
import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference
import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
import java.lang.IllegalStateException
class PropertyInitializationInfo(
map: PersistentMap<FirPropertySymbol, EventOccurrencesRange> = persistentMapOf()
@@ -52,17 +52,97 @@ class LocalPropertyCollector private constructor() : ControlFlowGraphVisitorVoid
}
}
class PathAwarePropertyInitializationInfo(
map: PersistentMap<EdgeLabel, PropertyInitializationInfo> = persistentMapOf()
) : ControlFlowInfo<PathAwarePropertyInitializationInfo, EdgeLabel, PropertyInitializationInfo>(map) {
companion object {
val EMPTY = PathAwarePropertyInitializationInfo(persistentMapOf(NormalPath to PropertyInitializationInfo.EMPTY))
}
override val constructor: (PersistentMap<EdgeLabel, PropertyInitializationInfo>) -> PathAwarePropertyInitializationInfo =
::PathAwarePropertyInitializationInfo
val infoAtNormalPath: PropertyInitializationInfo
get() = map[NormalPath] ?: PropertyInitializationInfo.EMPTY
val hasNormalPath: Boolean
get() = map.containsKey(NormalPath)
fun applyLabel(node: CFGNode<*>, label: EdgeLabel): PathAwarePropertyInitializationInfo {
if (label.isNormal) {
// Special case: when we exit the try expression, null label means a normal path.
// Filter out any info bound to non-null label
// One day, if we allow multiple edges between nodes with different labels, e.g., labeling all paths in try/catch/finally,
// instead of this kind of special handling, proxy enter/exit nodes per label are preferred.
if (node is TryExpressionExitNode) {
return if (hasNormalPath) {
constructor(persistentMapOf(NormalPath to infoAtNormalPath))
} else {
/* This means no info for normal path. */
EMPTY
}
}
// In general, null label means no additional path info, hence return `this` as-is.
return this
}
val hasAbnormalLabels = map.keys.any { !it.isNormal }
return if (hasAbnormalLabels) {
// { |-> ... l1 |-> I1, l2 |-> I2, ... }
// | l1 // path exit: if the given info has non-null labels, this acts like a filtering
// { |-> I1 } // NB: remove the path info
if (map.keys.contains(label)) {
constructor(persistentMapOf(NormalPath to map[label]!!))
} else {
/* This means no info for the specific label. */
EMPTY
}
} else {
// { |-> ... } // empty path info
// | l1 // path entry
// { l1 -> ... } // now, every info bound to the label
constructor(persistentMapOf(label to infoAtNormalPath))
}
}
fun merge(other: PathAwarePropertyInitializationInfo): PathAwarePropertyInitializationInfo {
var resultMap = persistentMapOf<EdgeLabel, PropertyInitializationInfo>()
for (label in keys.union(other.keys)) {
// disjoint merging to preserve paths. i.e., merge the property initialization info if and only if both have the key.
// merge({ |-> I1 }, { |-> I2, l1 |-> I3 }
// == { |-> merge(I1, I2), l1 |-> I3 }
val i1 = this[label]
val i2 = other[label]
resultMap = when {
i1 != null && i2 != null ->
resultMap.put(label, i1.merge(i2))
i1 != null ->
resultMap.put(label, i1)
i2 != null ->
resultMap.put(label, i2)
else ->
throw IllegalStateException()
}
}
return constructor(resultMap)
}
}
class PropertyInitializationInfoCollector(private val localProperties: Set<FirPropertySymbol>) :
ControlFlowGraphVisitor<PropertyInitializationInfo, Collection<PropertyInitializationInfo>>() {
override fun visitNode(node: CFGNode<*>, data: Collection<PropertyInitializationInfo>): PropertyInitializationInfo {
if (data.isEmpty()) return PropertyInitializationInfo.EMPTY
return data.reduce(PropertyInitializationInfo::merge)
ControlFlowGraphVisitor<PathAwarePropertyInitializationInfo, Collection<Pair<EdgeLabel, PathAwarePropertyInitializationInfo>>>() {
override fun visitNode(
node: CFGNode<*>,
data: Collection<Pair<EdgeLabel, PathAwarePropertyInitializationInfo>>
): PathAwarePropertyInitializationInfo {
if (data.isEmpty()) return PathAwarePropertyInitializationInfo.EMPTY
return data.map { (label, info) -> info.applyLabel(node, label) }
.reduce(PathAwarePropertyInitializationInfo::merge)
}
override fun visitVariableAssignmentNode(
node: VariableAssignmentNode,
data: Collection<PropertyInitializationInfo>
): PropertyInitializationInfo {
data: Collection<Pair<EdgeLabel, PathAwarePropertyInitializationInfo>>
): PathAwarePropertyInitializationInfo {
val dataForNode = visitNode(node, data)
val reference = node.fir.lValue as? FirResolvedNamedReference ?: return dataForNode
val symbol = reference.resolvedSymbol as? FirPropertySymbol ?: return dataForNode
@@ -75,8 +155,8 @@ class PropertyInitializationInfoCollector(private val localProperties: Set<FirPr
override fun visitVariableDeclarationNode(
node: VariableDeclarationNode,
data: Collection<PropertyInitializationInfo>
): PropertyInitializationInfo {
data: Collection<Pair<EdgeLabel, PathAwarePropertyInitializationInfo>>
): PathAwarePropertyInitializationInfo {
val dataForNode = visitNode(node, data)
return if (node.fir.initializer == null && node.fir.delegate == null) {
dataForNode
@@ -86,18 +166,27 @@ class PropertyInitializationInfoCollector(private val localProperties: Set<FirPr
}
fun getData(graph: ControlFlowGraph) =
graph.collectDataForNode(
graph.collectPathAwareDataForNode(
TraverseDirection.Forward,
PropertyInitializationInfo.EMPTY,
PathAwarePropertyInitializationInfo.EMPTY,
this
)
private fun processVariableWithAssignment(
dataForNode: PropertyInitializationInfo,
dataForNode: PathAwarePropertyInitializationInfo,
symbol: FirPropertySymbol
): PropertyInitializationInfo {
val existingKind = dataForNode[symbol] ?: EventOccurrencesRange.ZERO
val kind = existingKind + EventOccurrencesRange.EXACTLY_ONCE
return dataForNode.put(symbol, kind)
): PathAwarePropertyInitializationInfo {
assert(dataForNode.keys.isNotEmpty())
var resultMap = persistentMapOf<EdgeLabel, PropertyInitializationInfo>()
// before: { |-> { p1 |-> PI1 }, l1 |-> { p2 |-> PI2 }
for (label in dataForNode.keys) {
val dataPerLabel = dataForNode[label]!!
val existingKind = dataPerLabel[symbol] ?: EventOccurrencesRange.ZERO
val kind = existingKind + EventOccurrencesRange.EXACTLY_ONCE
resultMap = resultMap.put(label, dataPerLabel.put(symbol, kind))
}
// after (if symbol is p1):
// { |-> { p1 |-> PI1 + 1 }, l1 |-> { p1 |-> [1, 1], p2 |-> PI2 }
return PathAwarePropertyInitializationInfo(resultMap)
}
}
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt
+65
View File
@@ -7,6 +7,8 @@ package org.jetbrains.kotlin.fir.analysis.cfa
import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
// ------------------------------ Graph Traversal ------------------------------
enum class TraverseDirection {
Forward, Backward
}
@@ -29,6 +31,10 @@ fun ControlFlowGraph.traverse(
traverse(direction, visitor, null)
}
// --------------------- Path-insensitive data collection ----------------------
// TODO: Deprecate and make all existing checkers path-sensitive
fun <I : ControlFlowInfo<I, K, V>, K : Any, V : Any> ControlFlowGraph.collectDataForNode(
direction: TraverseDirection,
initialInfo: I,
@@ -75,3 +81,62 @@ private fun <I : ControlFlowInfo<I, K, V>, K : Any, V : Any> ControlFlowGraph.co
}
}
}
// ---------------------- Path-sensitive data collection -----------------------
// TODO: Once migration is done, get rid of "PathAware" from util names
fun <I : ControlFlowInfo<I, K, V>, K : Any, V : Any> ControlFlowGraph.collectPathAwareDataForNode(
direction: TraverseDirection,
initialInfo: I,
visitor: ControlFlowGraphVisitor<I, Collection<Pair<EdgeLabel, I>>>
): Map<CFGNode<*>, I> {
val nodeMap = LinkedHashMap<CFGNode<*>, I>()
val startNode = getEnterNode(direction)
nodeMap[startNode] = initialInfo
val changed = mutableMapOf<CFGNode<*>, Boolean>()
do {
collectPathAwareDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed)
} while (changed.any { it.value })
return nodeMap
}
private fun <I : ControlFlowInfo<I, K, V>, K : Any, V : Any> ControlFlowGraph.collectPathAwareDataForNodeInternal(
direction: TraverseDirection,
initialInfo: I,
visitor: ControlFlowGraphVisitor<I, Collection<Pair<EdgeLabel, I>>>,
nodeMap: MutableMap<CFGNode<*>, I>,
changed: MutableMap<CFGNode<*>, Boolean>
) {
val nodes = getNodesInOrder(direction)
for (node in nodes) {
if (direction == TraverseDirection.Backward && node is CFGNodeWithCfgOwner<*>) {
node.subGraphs.forEach { it.collectPathAwareDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) }
}
val previousNodes = when (direction) {
TraverseDirection.Forward -> node.previousCfgNodes
TraverseDirection.Backward -> node.followingCfgNodes
}
// One noticeable different against the path-unaware version is, here, we pair the control-flow info with the label.
val previousData =
previousNodes.mapNotNull {
val k = when (direction) {
TraverseDirection.Forward -> node.incomingEdges[it]?.label ?: NormalPath
TraverseDirection.Backward -> node.outgoingEdges[it]?.label ?: NormalPath
}
val v = nodeMap[it] ?: return@mapNotNull null
k to v
}
val data = nodeMap[node]
val newData = node.accept(visitor, previousData)
val hasChanged = newData != data
changed[node] = hasChanged
if (hasChanged) {
nodeMap[node] = newData
}
if (direction == TraverseDirection.Forward && node is CFGNodeWithCfgOwner<*>) {
node.subGraphs.forEach { it.collectPathAwareDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) }
}
}
}
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt
+18 -6
View File
@@ -22,7 +22,7 @@ object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChec
override fun analyze(
graph: ControlFlowGraph,
reporter: DiagnosticReporter,
data: Map<CFGNode<*>, PropertyInitializationInfo>,
data: Map<CFGNode<*>, PathAwarePropertyInitializationInfo>,
properties: Set<FirPropertySymbol>
) {
val localData = data.filter {
@@ -37,7 +37,7 @@ object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChec
}
private class UninitializedPropertyReporter(
val data: Map<CFGNode<*>, PropertyInitializationInfo>,
val data: Map<CFGNode<*>, PathAwarePropertyInitializationInfo>,
val localProperties: Set<FirPropertySymbol>,
val reporter: DiagnosticReporter
) : ControlFlowGraphVisitorVoid() {
@@ -48,12 +48,24 @@ object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChec
val symbol = reference.resolvedSymbol as? FirPropertySymbol ?: return
if (symbol !in localProperties) return
if (symbol.fir.isLateInit) return
val kind = data.getValue(node)[symbol] ?: EventOccurrencesRange.ZERO
if (!kind.isDefinitelyVisited()) {
node.fir.source?.let {
reporter.report(FirErrors.UNINITIALIZED_VARIABLE.on(it, symbol))
val pathAwareInfo = data.getValue(node)
for (label in pathAwareInfo.keys) {
if (investigate(pathAwareInfo[label]!!, symbol, node)) {
// To avoid duplicate reports, stop investigating remaining paths if the property is not initialized at any path.
break
}
}
}
private fun investigate(info: PropertyInitializationInfo, symbol: FirPropertySymbol, node: QualifiedAccessNode): Boolean {
val kind = info[symbol] ?: EventOccurrencesRange.ZERO
if (!kind.isDefinitelyVisited()) {
node.fir.source?.let {
reporter.report(FirErrors.UNINITIALIZED_VARIABLE.on(it, symbol))
return true
}
}
return false
}
}
}
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt
+5 -9
View File
@@ -5,16 +5,12 @@
package org.jetbrains.kotlin.fir.analysis.checkers.extended
import com.intellij.lang.LighterASTNode
import com.intellij.openapi.util.Ref
import org.jetbrains.kotlin.KtNodeTypes
import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange
import org.jetbrains.kotlin.fir.*
import org.jetbrains.kotlin.fir.analysis.cfa.AbstractFirPropertyInitializationChecker
import org.jetbrains.kotlin.fir.analysis.cfa.PropertyInitializationInfo
import org.jetbrains.kotlin.fir.analysis.cfa.TraverseDirection
import org.jetbrains.kotlin.fir.analysis.cfa.traverse
import org.jetbrains.kotlin.fir.analysis.cfa.*
import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
import org.jetbrains.kotlin.fir.analysis.getChild
@@ -23,12 +19,11 @@ import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
import org.jetbrains.kotlin.lexer.KtTokens
object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
override fun analyze(
graph: ControlFlowGraph,
reporter: DiagnosticReporter,
data: Map<CFGNode<*>, PropertyInitializationInfo>,
data: Map<CFGNode<*>, PathAwarePropertyInitializationInfo>,
properties: Set<FirPropertySymbol>
) {
val unprocessedProperties = mutableSetOf<FirPropertySymbol>()
@@ -75,7 +70,7 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
value in canBeValOccurrenceRanges && symbol.fir.isVar
private class UninitializedPropertyReporter(
val data: Map<CFGNode<*>, PropertyInitializationInfo>,
val data: Map<CFGNode<*>, PathAwarePropertyInitializationInfo>,
val localProperties: Set<FirPropertySymbol>,
val unprocessedProperties: MutableSet<FirPropertySymbol>,
val propertiesCharacteristics: MutableMap<FirPropertySymbol, EventOccurrencesRange>
@@ -89,7 +84,8 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
unprocessedProperties.remove(symbol)
val currentCharacteristic = propertiesCharacteristics.getOrDefault(symbol, EventOccurrencesRange.ZERO)
propertiesCharacteristics[symbol] = currentCharacteristic.or(data.getValue(node)[symbol] ?: EventOccurrencesRange.ZERO)
val info = data.getValue(node)
propertiesCharacteristics[symbol] = currentCharacteristic.or(info.infoAtNormalPath[symbol] ?: EventOccurrencesRange.ZERO)
}
override fun visitVariableDeclarationNode(node: VariableDeclarationNode) {
compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithCatch.fir.kt
Vendored
+2 -2
View File
@@ -27,7 +27,7 @@ fun assignedInTryAndCatch() {
a = 41
} finally {
}
<!UNINITIALIZED_VARIABLE!>a<!>.hashCode()
a.hashCode()
}
fun sideEffectBeforeAssignedInTryAndCatch(s: Any) {
@@ -40,7 +40,7 @@ fun sideEffectBeforeAssignedInTryAndCatch(s: Any) {
a = 41
} finally {
}
<!UNINITIALIZED_VARIABLE!>a<!>.hashCode()
a.hashCode()
}
fun assignedAtAll() {
compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithoutCatch.fir.kt
Vendored
+2 -2
View File
@@ -4,7 +4,7 @@ fun assignedInTry() {
a = 42
} finally {
}
<!UNINITIALIZED_VARIABLE!>a<!>.hashCode()
a.hashCode()
}
fun sideEffectBeforeAssignmentInTry(s: Any) {
@@ -14,7 +14,7 @@ fun sideEffectBeforeAssignmentInTry(s: Any) {
a = 42
} finally {
}
<!UNINITIALIZED_VARIABLE!>a<!>.hashCode()
a.hashCode()
}
fun assignedInTryAndFinally() {
compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/controlFlow/initialization/neg/3.fir.kt
Vendored
+1 -1
View File
@@ -95,7 +95,7 @@ fun case_6() {
println(<!UNINITIALIZED_VARIABLE!>value_2<!>.inc())
}
<!UNINITIALIZED_VARIABLE!>value_2<!>++
value_2++
}
// TESTCASE NUMBER: 7