From d46075a9dbc7cc85cd3f5ff4d0179ed36180382d Mon Sep 17 00:00:00 2001 From: Simon Ogorodnik Date: Fri, 1 Dec 2023 23:09:06 +0100 Subject: [PATCH] K2: Use correct position for substitution constrains in builder inference Before, we were wrapping the original constraint position into ConeBuilderInferenceSubstitutionConstraintPosition twice during the constraint substitution in builder inference. It was causing problems with diagnostic reporting. Remove TODOs about diagnostic reporting from FirBuilderInferenceSession. Propagation of errors from the common system is now implemented Diagnostic reporting in updateCalls isn't needed, since we report errors after the system completion unlike K1 Relates to KT-53749 --- .../inference/FirBuilderInferenceSession.kt | 29 ++++++++++--------- .../constraints/violating.fir.kt | 4 +-- .../buildListRemoveAddInBranches.fir.kt | 6 ++-- .../inconsistentTypeInference.fir.diag.txt | 2 +- .../inconsistentTypeInference2.fir.diag.txt | 2 +- .../builderInference/unsafeAssignment.fir.kt | 2 +- .../unsafeAssignmentExtra.fir.kt | 12 ++++---- .../upperBoundViolation.fir.diag.txt | 2 +- .../suspendCallsWrongUpperBound.fir.kt | 6 ++-- 9 files changed, 33 insertions(+), 32 deletions(-) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/FirBuilderInferenceSession.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/FirBuilderInferenceSession.kt index f29c29240bf..acd8ece6195 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/FirBuilderInferenceSession.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/inference/FirBuilderInferenceSession.kt @@ -174,11 +174,6 @@ class FirBuilderInferenceSession( if (hasConstraints) effectivelyEmptyCommonSystem = false } - // TODO: add diagnostics holder -// for (diagnostic in diagnostics) { -// commonSystem.addError(diagnostic) -// } - return commonSystem to effectivelyEmptyCommonSystem } @@ -252,11 +247,9 @@ class FirBuilderInferenceSession( receiver.updateTypeInBuilderInference(substitutor.substituteOrSelf(receiver.type)) } - // TODO: support diagnostics, see [CoroutineInferenceSession#updateCalls] val completionResultsWriter = components.callCompleter.createCompletionResultsWriter(substitutor) for ((call, _) in partiallyResolvedCalls) { call.transformSingle(completionResultsWriter, null) - // TODO: support diagnostics, see [CoroutineInferenceSession#updateCalls] } } @@ -307,12 +300,15 @@ class FirBuilderInferenceSession( substitutor: TypeSubstitutorMarker, fixedTypeVariables: Map ): InitialConstraint { - val substituted = substitute(substitutor) + // TODO: invalid naming. It doesn't make sense case of equality constraints + val substitutedLowerType = substitutor.safeSubstitute(resolutionContext.typeContext, this.a) + val substitutedUpperType = substitutor.safeSubstitute(resolutionContext.typeContext, this.b) + val a = a // In situation when some type variable _T is fixed to Stub(_T)?, // we are not allowed just to substitute Stub(_T) with T because nullabilities are different here! // To compensate this, we have to substitute Stub(_T) <: SomeType constraint with T <: SomeType? adding nullability to upper type - if (a is ConeStubTypeForChainInference && substituted.a !is ConeStubTypeForChainInference) { + if (a is ConeStubTypeForChainInference && substitutedLowerType !is ConeStubTypeForChainInference) { val constructor = a.constructor val fixedTypeVariableType = fixedTypeVariables[constructor.variable.typeConstructor] if (fixedTypeVariableType is ConeStubTypeForChainInference && @@ -320,14 +316,19 @@ class FirBuilderInferenceSession( fixedTypeVariableType.isMarkedNullable ) { return InitialConstraint( - substituted.a, - (substituted.b as ConeKotlinType).withNullability(ConeNullability.NULLABLE, resolutionContext.typeContext), - substituted.constraintKind, - substituted.position + substitutedLowerType, + (substitutedUpperType as ConeKotlinType).withNullability(ConeNullability.NULLABLE, resolutionContext.typeContext), + constraintKind, + position ) } } - return substituted + return InitialConstraint( + substitutedLowerType, + substitutedUpperType, + constraintKind, + position + ) } } diff --git a/compiler/testData/diagnostics/tests/inference/builderInference/constraints/violating.fir.kt b/compiler/testData/diagnostics/tests/inference/builderInference/constraints/violating.fir.kt index 684db09bf6c..5d0a8aa932f 100644 --- a/compiler/testData/diagnostics/tests/inference/builderInference/constraints/violating.fir.kt +++ b/compiler/testData/diagnostics/tests/inference/builderInference/constraints/violating.fir.kt @@ -13,7 +13,7 @@ fun main() { override fun foo(): MutableList = this@buildList } } - buildList { + buildList { add(3) val x: String = get(0) } @@ -26,7 +26,7 @@ fun main() { add(y) val x: MutableList = this@buildList } - buildList { + buildList { add("") val x: StringBuilder = get(0) } diff --git a/compiler/testData/diagnostics/testsWithStdLib/builderInference/buildListRemoveAddInBranches.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/builderInference/buildListRemoveAddInBranches.fir.kt index 59d01212ac0..c0b118ece0a 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/builderInference/buildListRemoveAddInBranches.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/builderInference/buildListRemoveAddInBranches.fir.kt @@ -1,8 +1,8 @@ // ISSUE: KT-55168 -fun foo(arg: Boolean) = buildList { +fun foo(arg: Boolean) = buildList { if (arg) { removeLast() } else { - add(42) + add(42) } -} +} diff --git a/compiler/testData/diagnostics/testsWithStdLib/builderInference/inconsistentTypeInference.fir.diag.txt b/compiler/testData/diagnostics/testsWithStdLib/builderInference/inconsistentTypeInference.fir.diag.txt index c66826a0ea9..d582fb5e955 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/builderInference/inconsistentTypeInference.fir.diag.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/builderInference/inconsistentTypeInference.fir.diag.txt @@ -1 +1 @@ -/inconsistentTypeInference.fir.kt:(62,127): error: New inference error [NewConstraintError at Incorporate kotlin/collections/MutableList <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call into some call from position Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call into some call: kotlin/String <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call from position Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call: kotlin/String <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call into some call from position Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call into some call: kotlin/String <: kotlin/collections/Collection from Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call from position Incorporated builder inference constraint kotlin/collections/MutableList <: kotlin/collections/Collection from Receiver this@R|special/anonymous| into some call: kotlin/String myBuilder(block: Foo.() -> Unit) : Foo = Foo().apply(bloc fun main(arg: Any) { val x = 57 - val value = myBuilder { + val value = myBuilder { doSmthng("one ") run { a; this }.a = 10 a += 1 diff --git a/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.kt b/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.kt index cc39de190c8..5cb4a041fc5 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/builderInference/unsafeAssignmentExtra.fir.kt @@ -60,18 +60,18 @@ fun main(arg: Any, condition: Boolean) { } // See KT-54664 - val value3 = myBuilder { + val value3 = myBuilder { accept("") a = 45 - bar(::a) - } + bar(::a) + } fun baz(t: Int) {} - val value4 = myBuilder { + val value4 = myBuilder { accept("") a = 45 b[0] = 123 - baz(a) - } + baz(a) + } } diff --git a/compiler/testData/diagnostics/testsWithStdLib/builderInference/upperBoundViolation.fir.diag.txt b/compiler/testData/diagnostics/testsWithStdLib/builderInference/upperBoundViolation.fir.diag.txt index 43456d803d3..85dd7cd9988 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/builderInference/upperBoundViolation.fir.diag.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/builderInference/upperBoundViolation.fir.diag.txt @@ -1 +1 @@ -/upperBoundViolation.fir.kt:(150,260): error: New inference error [NewConstraintError at Incorporate TypeVariable(E) <: kotlin/Number from Incorporated builder inference constraint Stub (chain inference): TypeVariable(E) <: kotlin/Number from Incorporated builder inference constraint TypeVariable(T) <: kotlin/Number from DeclaredUpperBound into some call into some call from position Incorporated builder inference constraint Stub (chain inference): TypeVariable(E) <: kotlin/Number from Incorporated builder inference constraint TypeVariable(T) <: kotlin/Number from DeclaredUpperBound into some call into some call: kotlin/String { fun generate(g: suspend Controller.() -> Unit): S = TODO() -val test = generate { - yield("foo") -} +val test = generate { + yield("foo") +}