[FIR] Attach YT tickets to meaningful TODOs in :fir:resolve module

This commit is contained in:
Dmitriy Novozhilov
2023-06-27 16:24:32 +03:00
committed by Space Team
parent 22b2774ef7
commit 10054c584c
14 changed files with 28 additions and 23 deletions
@@ -254,7 +254,8 @@ class FirSamResolver(
return resolvedFunctionType.getOrPut(firRegularClass) {
if (!firRegularClass.status.isFun) return@getOrPut null
val abstractMethod = firRegularClass.getSingleAbstractMethodOrNull(session, scopeSession) ?: return@getOrPut null
// TODO: val shouldConvertFirstParameterToDescriptor = samWithReceiverResolvers.any { it.shouldConvertFirstSamParameterToReceiver(abstractMethod) }
// TODO: KT-59674
// val shouldConvertFirstParameterToDescriptor = samWithReceiverResolvers.any { it.shouldConvertFirstSamParameterToReceiver(abstractMethod) }
val typeFromExtension = samConversionTransformers.firstNotNullOfOrNull {
it.getCustomFunctionTypeForSamConversion(abstractMethod)
@@ -588,7 +588,7 @@ internal object CheckIncompatibleTypeVariableUpperBounds : ResolutionStage() {
for (variableWithConstraints in candidate.system.notFixedTypeVariables.values) {
val upperTypes = variableWithConstraints.constraints.extractUpperTypesToCheckIntersectionEmptiness()
// TODO: consider reporting errors on bounded type variables by incompatible types but with other lower constraints
// TODO: consider reporting errors on bounded type variables by incompatible types but with other lower constraints, KT-59676
if (upperTypes.size <= 1 || variableWithConstraints.constraints.any { it.kind.isLower() })
continue
@@ -102,7 +102,7 @@ class FirTowerResolver(
if (outerType != null)
components.implicitReceiverStack.receiversAsReversed().drop(1).firstOrNull {
AbstractTypeChecker.isSubtypeOf(components.session.typeContext, it.type, outerType)
} ?: return collector // TODO: report diagnostic about not-found receiver
} ?: return collector // TODO: report diagnostic about not-found receiver, KT-59677
else
null
@@ -652,7 +652,7 @@ abstract class FirDataFlowAnalyzer(
private fun enterRepeatableStatement(flow: MutableFlow, statement: FirStatement) {
val reassignedNames = context.preliminaryLoopVisitor.enterCapturingStatement(statement)
if (reassignedNames.isEmpty()) return
// TODO: only choose the innermost variable for each name
// TODO: only choose the innermost variable for each name, KT-59688
val possiblyChangedVariables = variableStorage.realVariables.values.filter {
val identifier = it.identifier
val symbol = identifier.symbol
@@ -754,6 +754,7 @@ abstract class FirDataFlowAnalyzer(
val expressionVariable = variableStorage.getOrCreate(flow, safeCall)
// TODO? all new implications in previous node's flow are valid here if receiver != null
// (that requires a second level of implications: receiver != null => condition => effect).
// KT-59689
flow.addAllConditionally(expressionVariable notEq null, node.lastPreviousNode.flow)
}
}
@@ -1000,7 +1001,7 @@ abstract class FirDataFlowAnalyzer(
// Approved type statements for RHS already contain everything implied by the corresponding value of LHS.
val bothEvaluated = operatorVariable eq isAnd
// TODO? `bothEvaluated` also implies all implications from RHS. This requires a second level
// of implications, which the logic system currently doesn't support. See also safe calls.
// of implications, which the logic system currently doesn't support. See also safe calls. KT-59689
flow.addAllConditionally(bothEvaluated, flowFromRight)
if (rightIsBoolean) {
flow.addAllConditionally(bothEvaluated, logicSystem.approveOperationStatement(flowFromRight, rightVariable!! eq isAnd))
@@ -1014,7 +1015,7 @@ abstract class FirDataFlowAnalyzer(
// Not checking for reassignments is safe since we will only take statements that are also true in RHS
// (so they're true regardless of whether the variable ends up being reassigned or not).
logicSystem.approveOperationStatement(flowFromLeft, leftVariable!! eq !isAnd),
// TODO: and(approved from right, ...)? FE1.0 doesn't seem to handle that correctly either.
// TODO: and(approved from right, ...)? FE1.0 doesn't seem to handle that correctly either. KT-59690
// if (x is A || whatever(x as B)) { /* x is (A | B) */ }
logicSystem.approveOperationStatement(flowFromRight, rightVariable!! eq !isAnd),
)
@@ -99,6 +99,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
// p.memberOfSomething // Bad
// }
// FE1.0 has the same behavior.
// KT-59692
currentInfo?.assignedInside?.let(prohibitInOuterScope::addAll)
// => any write to a variable outside the callable invalidates smart casts inside it
outerInfo?.assignedLater?.let(prohibitInThisScope::addAll)
@@ -348,7 +349,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
// TODO: liveness analysis - return/throw/break/continue terminate the flow.
// This is somewhat problematic though because try-catch and loops can restore it.
// It is not possible to implement liveness analysis partially - otherwise combinations of
// control flow structures can cause incorrect smartcasts.
// control flow structures can cause incorrect smartcasts. KT-59691
override fun visitFunctionCall(functionCall: FirFunctionCall, data: MiniCfgData) {
val visitor = this
@@ -356,7 +357,7 @@ internal class FirLocalVariableAssignmentAnalyzer {
setOfNotNull(explicitReceiver, dispatchReceiver, extensionReceiver).forEach { it.accept(visitor, data) }
// Delay processing of lambda args because lambda body are evaluated after all arguments have been evaluated.
// TODO: this is not entirely correct (the lambda might be nested deep inside an expression), but also this
// entire override should be unnecessary as long as the full CFG builder visits everything in the right order
// entire override should be unnecessary as long as the full CFG builder visits everything in the right order. KT-59691
val (postponedFunctionArgs, normalArgs) = argumentList.arguments.partition { it is FirAnonymousFunctionExpression }
normalArgs.forEach { it.accept(visitor, data) }
postponedFunctionArgs.forEach { it.accept(visitor, data) }
@@ -341,7 +341,7 @@ class ControlFlowGraphBuilder {
//
// TODO: an alternative is to delay computing incoming flow for "branch result exit" nodes
// until the entire "when" is resolved; then either unify each branch's lambdas into its
// exit node, or create N union nodes (1/branch) and point them into the merge node.
// exit node, or create N union nodes (1/branch) and point them into the merge node. KT-59730
private fun mergeDataFlowFromPostponedLambdas(node: CFGNode<*>, callCompleted: Boolean) {
val currentLevelExits = postponedLambdaExits.pop()
if (currentLevelExits.isEmpty()) return
@@ -445,7 +445,7 @@ class ControlFlowGraphBuilder {
if ((klass as FirControlFlowGraphOwner).controlFlowGraphReference != null) {
// TODO: IDE LL API sometimes attempts to analyze a enum class while already analyzing it, causing
// this graph to be built twice (or more). Not sure what this means. Nothing good, probably.
// In any case, attempting to add more edges to subgraphs will be fatal.
// In any case, attempting to add more edges to subgraphs will be fatal. KT-59728
graphs.pop()
return null to null
}
@@ -661,7 +661,7 @@ class ControlFlowGraphBuilder {
if (jump is FirReturnExpression && jump.target.labeledElement is FirAnonymousFunction) {
// TODO: these should be DFA-only edges; they should be pointed into the postponed function exit node?
// With builder inference, lambdas are not necessarily resolved starting from the innermost one...
// See analysis test cfg/postponedLambdaInReturn.kt.
// See analysis test cfg/postponedLambdaInReturn.kt. KT-59729
postponedLambdaExits.pop()
}
@@ -973,8 +973,9 @@ class ControlFlowGraphBuilder {
// } finally {}
// }
// try {} finally { /* return@x target is in nonDirectJumps */ }
// KT-59725
node.level < minLevel || node !in nonDirectJumps -> continue
// TODO: if the input to finally with that label is dead, then so should be the exit probably
// TODO: if the input to finally with that label is dead, then so should be the exit probably. KT-59725
node.returnPathIsBackwards -> addBackEdge(this, node, label = node)
else -> addEdge(this, node, propagateDeadness = false, label = node)
}
@@ -1029,6 +1030,7 @@ class ControlFlowGraphBuilder {
// TODO: this doesn't make fully 'right' Nothing node (doesn't support going to catch and pass through finally)
// because doing those afterwards is quite challenging
// it would be much easier if we could build calls after full completion only, at least for Nothing calls
// KT-59726
// @returns `true` if node actually returned Nothing
private fun completeFunctionCall(node: FunctionCallNode): Boolean {
if (!node.fir.resultType.isNothing) return false
@@ -34,7 +34,7 @@ object ConeConstraintSystemUtilContext : ConstraintSystemUtilContext {
override fun KotlinTypeMarker.unCapture(): KotlinTypeMarker {
require(this is ConeKotlinType)
// TODO, see TypeUtils.kt
// TODO, see TypeUtils.kt, KT-59678
return this
}
@@ -67,7 +67,7 @@ class ConstraintSystemCompleter(components: BodyResolveComponents, private val c
}
completion@ while (true) {
// TODO: This is very slow
// TODO: This is very slow, KT-59680
val postponedArguments = getOrderedNotAnalyzedPostponedArguments(topLevelAtoms)
if (completionMode == ConstraintSystemCompletionMode.UNTIL_FIRST_LAMBDA && hasLambdaToAnalyze(
@@ -409,7 +409,7 @@ class ConstraintSystemCompleter(components: BodyResolveComponents, private val c
val notAnalyzedArguments = arrayListOf<PostponedResolvedAtom>()
for (primitive in topLevelAtoms) {
primitive.processAllContainingCallCandidates(
// TODO: remove this argument and relevant parameter
// TODO: remove this argument and relevant parameter, KT-59679
// Currently, it's used because otherwise problem happens with a lambda in a try-block (see tryWithLambdaInside test)
processBlocks = true
) { candidate ->
@@ -799,7 +799,7 @@ class FirCallCompletionResultsWriterTransformer(
return varargArgumentsExpression
}
// TODO: report warning with a checker and return true here only in case of errors
// TODO: report warning with a checker and return true here only in case of errors, KT-59676
private fun FirNamedReferenceWithCandidate.hasAdditionalResolutionErrors(): Boolean =
candidate.system.errors.any { it is InferredEmptyIntersection }
@@ -848,4 +848,4 @@ private fun ExpectedArgumentType.getExpectedType(argument: FirElement): ConeKotl
ExpectedArgumentType.NoApproximation -> null
}
fun ConeKotlinType.toExpectedType(): ExpectedArgumentType = ExpectedArgumentType.ExpectedType(this)
fun ConeKotlinType.toExpectedType(): ExpectedArgumentType = ExpectedArgumentType.ExpectedType(this)
@@ -36,7 +36,7 @@ fun FirFunctionCall.replaceLambdaArgumentInvocationKinds(session: FirSession) {
val byParameter = mutableMapOf<FirValueParameter, EventOccurrencesRange>()
function.contractDescription.effects?.forEach { fir ->
val effect = fir.effect as? ConeCallsEffectDeclaration ?: return@forEach
// TODO: Support callsInPlace contracts on receivers
// TODO: Support callsInPlace contracts on receivers, KT-59681
val valueParameter = function.valueParameters.getOrNull(effect.valueParameterReference.parameterIndex) ?: return@forEach
byParameter[valueParameter] = effect.kind
}
@@ -41,7 +41,7 @@ internal fun remapArgumentsWithVararg(
var indexAfterVarargs = argumentList.size
val newArgumentMapping = linkedMapOf<FirExpression, FirValueParameter>()
val varargArgument = buildVarargArgumentsExpression {
// TODO: ideally we should use here a source from the use-site and not from the declaration-site
// TODO: ideally we should use here a source from the use-site and not from the declaration-site, KT-59682
this.varargElementType = varargParameterTypeRef.withReplacedConeType(varargElementType, KtFakeSourceElementKind.VarargArgument)
this.typeRef = varargParameterTypeRef.withReplacedConeType(varargArrayType, KtFakeSourceElementKind.VarargArgument)
var startOffset = Int.MAX_VALUE
@@ -369,7 +369,7 @@ open class FirDeclarationsResolveTransformer(
// Resolve call for provideDelegate, without completion
// TODO: this generates some nodes in the control flow graph which we don't want if we
// end up not selecting this option.
// end up not selecting this option, KT-59684
transformer.expressionsTransformer.transformFunctionCallInternal(
provideDelegateCall, ResolutionMode.ContextIndependent, provideDelegate = true
)
@@ -554,7 +554,7 @@ open class FirDeclarationsResolveTransformer(
val result = context.withScopesForScript(script, components) {
transformDeclarationContent(script, data) as FirScript
}
dataFlowAnalyzer.exitScript() // TODO: FirScript should be a FirControlFlowGraphOwner
dataFlowAnalyzer.exitScript() // TODO: FirScript should be a FirControlFlowGraphOwner, KT-59683
return result
}
@@ -841,7 +841,7 @@ open class FirExpressionsResolveTransformer(transformer: FirAbstractBodyResolveT
private fun FirTypeRef.withTypeArgumentsForBareType(argument: FirExpression, operation: FirOperation): FirTypeRef {
val type = coneTypeSafe<ConeClassLikeType>() ?: return this
if (type.typeArguments.isNotEmpty()) return this // TODO: Incorrect for local classes.
if (type.typeArguments.isNotEmpty()) return this // TODO: Incorrect for local classes, KT-59686
// TODO: Check equality of size of arguments and parameters?
val firClass = type.lookupTag.toSymbol(session)?.fir ?: return this
@@ -93,7 +93,7 @@ object BuilderInferencePosition : ConstraintPosition() {
override fun toString(): String = "For builder inference call"
}
// TODO: should be used only in SimpleConstraintSystemImpl
// TODO: should be used only in SimpleConstraintSystemImpl, KT-59675
object SimpleConstraintSystemConstraintPosition : ConstraintPosition()
// ------------------------------------------------ Errors ------------------------------------------------