[FIR] Run CFA for member properties even if they have initializer

^KT-56678 Fixed
^KT-56682 Fixed
This commit is contained in:
Dmitriy Novozhilov
2023-02-15 12:31:26 +02:00
committed by Space Team
parent de84cb8038
commit c87e489dc9
28 changed files with 306 additions and 59 deletions
@@ -5702,6 +5702,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedMemberProperyWriteInInit.kt");
}
@Test
@TestMetadata("delegatedPropertyEarlyAccess.kt")
public void testDelegatedPropertyEarlyAccess() throws Exception {
@@ -6122,6 +6128,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryWithAssignmentUsedInCatch.kt");
}
@Test
@TestMetadata("uninintializedProperyWithDirectAndDelayedInitialization.kt")
public void testUninintializedProperyWithDirectAndDelayedInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/uninintializedProperyWithDirectAndDelayedInitialization.kt");
}
@Test
@TestMetadata("uninitializedCompanionOfEnum_after.kt")
public void testUninitializedCompanionOfEnum_after() throws Exception {
@@ -5696,6 +5696,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedMemberProperyWriteInInit.kt");
}
@Test
@TestMetadata("delegatedPropertyEarlyAccess.kt")
public void testDelegatedPropertyEarlyAccess() throws Exception {
@@ -6116,6 +6122,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/tryWithAssignmentUsedInCatch.kt");
}
@Test
@TestMetadata("uninintializedProperyWithDirectAndDelayedInitialization.kt")
public void testUninintializedProperyWithDirectAndDelayedInitialization() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/uninintializedProperyWithDirectAndDelayedInitialization.kt");
}
@Test
@TestMetadata("uninitializedCompanionOfEnum_after.kt")
public void testUninitializedCompanionOfEnum_after() throws Exception {
@@ -31,7 +31,7 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirSyntheticPropertySymbol
object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChecker() {
override fun analyze(data: PropertyInitializationInfoData, reporter: DiagnosticReporter, context: CheckerContext) {
data.checkPropertyAccesses(context, reporter)
data.checkPropertyAccesses(isForClassInitialization = false, context, reporter)
}
}
@@ -44,15 +44,36 @@ val FirDeclaration.evaluatedInPlace: Boolean
else -> true // property initializer, etc.
}
@OptIn(SymbolInternals::class)
val FirPropertySymbol.requiresInitialization: Boolean
get() = this !is FirSyntheticPropertySymbol && !hasInitializer && !hasExplicitBackingField &&
hasBackingField && fir.isCatchParameter != true
/**
* [isForClassInitialization] means that caller is interested in member property in the scope
* of class initialization section. In this case the fact that property has initializer does
* not mean that it's safe to access this property in any place:
*
* class A {
* val b = a // a is not initialized here
* val a = 10
* val c = a // but initialized here
* }
*/
fun PropertyInitializationInfoData.checkPropertyAccesses(context: CheckerContext, reporter: DiagnosticReporter) {
@OptIn(SymbolInternals::class)
fun FirPropertySymbol.requiresInitialization(isForClassInitialization: Boolean): Boolean {
val hasImplicitBackingField = !hasExplicitBackingField && hasBackingField
return when {
this is FirSyntheticPropertySymbol -> false
isForClassInitialization -> hasDelegate || hasImplicitBackingField
else -> !hasInitializer && hasImplicitBackingField && fir.isCatchParameter != true
}
}
fun PropertyInitializationInfoData.checkPropertyAccesses(
isForClassInitialization: Boolean,
context: CheckerContext,
reporter: DiagnosticReporter
) {
// If a property has an initializer (or does not need one), then any reads are OK while any writes are OK
// if it's a `var` and bad if it's a `val`. `FirReassignmentAndInvisibleSetterChecker` does this without a CFG.
val filtered = properties.filterTo(mutableSetOf()) { it.requiresInitialization }
val filtered = properties.filterTo(mutableSetOf()) { it.requiresInitialization(isForClassInitialization) }
if (filtered.isEmpty()) return
checkPropertyAccesses(graph, filtered, context, reporter, null, mutableMapOf())
@@ -73,6 +73,18 @@ class PropertyInitializationInfoCollector(
}
}
override fun visitPropertyInitializerExitNode(
node: PropertyInitializerExitNode,
data: PathAwareControlFlowInfo<PropertyInitializationInfo>
): PathAwareControlFlowInfo<PropertyInitializationInfo> {
// If member property initializer is empty (there are no nodes between enter and exit node)
// then property is not initialized in its declaration
// Otherwise it is
val dataForNode = visitNode(node, data)
if (node.firstPreviousNode is PropertyInitializerEnterNode) return dataForNode
return overwriteRange(dataForNode, node.fir.symbol, EventOccurrencesRange.EXACTLY_ONCE)
}
// --------------------------------------------------
// Data flows of declared/assigned variables in loops
// --------------------------------------------------
@@ -47,12 +47,12 @@ object FirMemberPropertiesChecker : FirClassChecker() {
private fun FirClass.collectInitializationInfo(context: CheckerContext, reporter: DiagnosticReporter): PropertyInitializationInfo? {
val graph = (this as? FirControlFlowGraphOwner)?.controlFlowGraphReference?.controlFlowGraph ?: return null
val memberPropertySymbols = declarations.mapNotNullTo(mutableSetOf()) {
(it.symbol as? FirPropertySymbol)?.takeIf { symbol -> symbol.requiresInitialization }
(it.symbol as? FirPropertySymbol)?.takeIf { symbol -> symbol.requiresInitialization(isForClassInitialization = true) }
}
if (memberPropertySymbols.isEmpty()) return null
// TODO: merge with `FirPropertyInitializationAnalyzer` for fewer passes.
val data = PropertyInitializationInfoData(memberPropertySymbols, symbol, graph)
data.checkPropertyAccesses(context, reporter)
data.checkPropertyAccesses(isForClassInitialization = true, context, reporter)
return data.getValue(graph.exitNode)[NormalPath]
}
@@ -138,7 +138,8 @@ object FirReassignmentAndInvisibleSetterChecker : FirVariableAssignmentChecker()
if (property.isVar) return
// Assignments of uninitialized `val`s must be checked via CFG, since the first one is OK.
// See `FirPropertyInitializationAnalyzer` for locals and `FirMemberPropertiesChecker` for backing fields in initializers.
if (property.requiresInitialization && (property.isLocal || isInOwnersInitializer(expression.dispatchReceiver, context))) return
if (property.isLocal && property.requiresInitialization(isForClassInitialization = false)) return
if (isInOwnersInitializer(expression.dispatchReceiver, context) && property.requiresInitialization(isForClassInitialization = true)) return
reporter.reportOn(expression.lValue.source, FirErrors.VAL_REASSIGNMENT, property, context)
}
@@ -43,7 +43,7 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
}
}
private class ReassignedVariableCollector(val data: PropertyInitializationInfoData, ) : ControlFlowGraphVisitorVoid() {
private class ReassignedVariableCollector(val data: PropertyInitializationInfoData) : ControlFlowGraphVisitorVoid() {
private val reassigned = mutableSetOf<FirPropertySymbol>()
override fun visitNode(node: CFGNode<*>) {}
@@ -51,7 +51,7 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() {
override fun visitVariableAssignmentNode(node: VariableAssignmentNode) {
val symbol = node.fir.calleeReference?.toResolvedPropertySymbol() ?: return
if (symbol.isVar && symbol.source?.kind !is KtFakeSourceElementKind && symbol in data.properties &&
(!symbol.requiresInitialization || data.getValue(node).values.any { it[symbol]?.canBeRevisited() == true })
(!symbol.requiresInitialization(isForClassInitialization = data.graph.kind == ControlFlowGraph.Kind.Class) || data.getValue(node).values.any { it[symbol]?.canBeRevisited() == true })
) {
reassigned.add(symbol)
}