From 10054c584ca74092fcdadbef82a303b8c05c1a6b Mon Sep 17 00:00:00 2001 From: Dmitriy Novozhilov Date: Tue, 27 Jun 2023 16:24:32 +0300 Subject: [PATCH] [FIR] Attach YT tickets to meaningful TODOs in :fir:resolve module --- .../org/jetbrains/kotlin/fir/resolve/SamResolution.kt | 3 ++- .../kotlin/fir/resolve/calls/ResolutionStages.kt | 2 +- .../kotlin/fir/resolve/calls/tower/FirTowerResolver.kt | 2 +- .../kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt | 7 ++++--- .../resolve/dfa/FirLocalVariableAssignmentAnalyzer.kt | 5 +++-- .../fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt | 10 ++++++---- .../inference/ConeConstraintSystemUtilContext.kt | 2 +- .../fir/resolve/inference/ConstraintSystemCompleter.kt | 4 ++-- .../FirCallCompletionResultsWriterTransformer.kt | 4 ++-- .../resolve/transformers/InvocationKindTransformer.kt | 2 +- .../transformers/body/resolve/BodyResolveUtils.kt | 2 +- .../body/resolve/FirDeclarationsResolveTransformer.kt | 4 ++-- .../body/resolve/FirExpressionsResolveTransformer.kt | 2 +- .../inference/model/ConstraintPositionAndErrors.kt | 2 +- 14 files changed, 28 insertions(+), 23 deletions(-) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/SamResolution.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/SamResolution.kt index c0a1020eb43..391607b9db7 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/SamResolution.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/SamResolution.kt @@ -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) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt index 371e672dbec..7e39cd4c847 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/ResolutionStages.kt @@ -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 diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolver.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolver.kt index d1ca4a4c47d..28b3e12bd46 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolver.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/tower/FirTowerResolver.kt @@ -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 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 3c687b7d351..4c87ad12e37 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 @@ -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), ) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirLocalVariableAssignmentAnalyzer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirLocalVariableAssignmentAnalyzer.kt index c9cdf3f26b0..1d763c56039 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirLocalVariableAssignmentAnalyzer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirLocalVariableAssignmentAnalyzer.kt @@ -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) } 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 11fa07e1182..65fe803e9d3 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 @@ -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 diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConeConstraintSystemUtilContext.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConeConstraintSystemUtilContext.kt index 12bcbbf9769..77acc18f4e4 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConeConstraintSystemUtilContext.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConeConstraintSystemUtilContext.kt @@ -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 } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConstraintSystemCompleter.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConstraintSystemCompleter.kt index d9d30009d54..ca16682037a 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConstraintSystemCompleter.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/ConstraintSystemCompleter.kt @@ -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() 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 -> diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirCallCompletionResultsWriterTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirCallCompletionResultsWriterTransformer.kt index cac4ea11117..8dd1668aaf5 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirCallCompletionResultsWriterTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/FirCallCompletionResultsWriterTransformer.kt @@ -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) \ No newline at end of file +fun ConeKotlinType.toExpectedType(): ExpectedArgumentType = ExpectedArgumentType.ExpectedType(this) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/InvocationKindTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/InvocationKindTransformer.kt index 0f3b3928595..f9c7e758dcb 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/InvocationKindTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/InvocationKindTransformer.kt @@ -36,7 +36,7 @@ fun FirFunctionCall.replaceLambdaArgumentInvocationKinds(session: FirSession) { val byParameter = mutableMapOf() 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 } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/BodyResolveUtils.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/BodyResolveUtils.kt index 6156e926c6a..012a8765da1 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/BodyResolveUtils.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/BodyResolveUtils.kt @@ -41,7 +41,7 @@ internal fun remapArgumentsWithVararg( var indexAfterVarargs = argumentList.size val newArgumentMapping = linkedMapOf() 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 diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt index cb1364b1f21..cb4d2b22ee2 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt @@ -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 } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt index bcb06d1b607..72baba44aab 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirExpressionsResolveTransformer.kt @@ -841,7 +841,7 @@ open class FirExpressionsResolveTransformer(transformer: FirAbstractBodyResolveT private fun FirTypeRef.withTypeArgumentsForBareType(argument: FirExpression, operation: FirOperation): FirTypeRef { val type = coneTypeSafe() ?: 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 diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/model/ConstraintPositionAndErrors.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/model/ConstraintPositionAndErrors.kt index 1df25b85be6..f6bda3f5942 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/model/ConstraintPositionAndErrors.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/model/ConstraintPositionAndErrors.kt @@ -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 ------------------------------------------------