From d04625666ad4b00e21b0f05bc3e81ce8e9baa320 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 1 Mar 2024 11:49:22 +0100 Subject: [PATCH] K2: don't approximate captured types in ResultTypeResolver in certain cases To be more precises, ResultTypeResolver for K2 now searches for similar LOWER/UPPER constraints pair based on the same flexible type, like LOWER(CapturedType&Any..CapturedType?) and UPPER(CapturedType!). If such a pair is found, the CapturedType is not approximated. This is done to avoid a big difference between this case and completely same constraints, like LOWER(CapturedType!) and UPPER(CapturedType!). In this case we squash them to EQUAL(CapturedType!) even before ResultTypeResolver, and captured types which is got from an EQUAL constraint are not approximated even before this commit. This commit fixes back a case from KT-50134 and a problem with intellij.vcs.git #KT-65596 Fixed --- .../resolve/inference/javaCollector.fir.txt | 2 +- .../resolve/inference/javaCollector.kt | 4 +- .../components/ResultTypeResolver.kt | 112 ++++++++++++------ .../tests/j+k/withTestConnection.fir.kt | 35 ------ .../tests/j+k/withTestConnection.kt | 1 + 5 files changed, 80 insertions(+), 74 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/j+k/withTestConnection.fir.kt diff --git a/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.fir.txt b/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.fir.txt index 67b7471a70f..b534c6daaa3 100644 --- a/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.fir.txt @@ -1,6 +1,6 @@ FILE: javaCollector.kt public final fun foo(): R|kotlin/Unit| { - R|kotlin/collections/listOf|(String()).R|SubstitutionOverride|>|().R|SubstitutionOverride#|<, >(Q|java/util/stream/Collectors|.R|java/util/stream/Collectors.groupingBy*s#|, R|kotlin/Any!|, R|kotlin/Int!|>(SAM(groupingBy@fun (it: R|@EnhancedNullability kotlin/String!|): { + R|kotlin/collections/listOf|(String()).R|SubstitutionOverride|>|().R|SubstitutionOverride|, kotlin/collections/Map?>|, R|ft|>(Q|java/util/stream/Collectors|.R|java/util/stream/Collectors.groupingBy*s||, R|kotlin/Int!|>(SAM(groupingBy@fun (it: R|@EnhancedNullability kotlin/String!|): R|@EnhancedNullability kotlin/String!| { ^ R|/it| } ), Q|java/util/stream/Collectors|.R|java/util/stream/Collectors.collectingAndThen*s||, R|kotlin/Long!|, R|kotlin/Int!|>(Q|java/util/stream/Collectors|.R|java/util/stream/Collectors.counting*s|(), SAM(Q|kotlin/Long|::R|kotlin/Long.toInt|)))) diff --git a/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.kt b/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.kt index 2d81d51e337..742ac2cf01b 100644 --- a/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.kt +++ b/compiler/fir/analysis-tests/testData/resolve/inference/javaCollector.kt @@ -6,12 +6,12 @@ import java.util.stream.Collectors fun foo(){ listOf("").stream().collect( - Collectors.groupingBy( + Collectors.groupingBy( { it }, Collectors.collectingAndThen( Collectors.counting(), Long::toInt ) - ) + ) ) } diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ResultTypeResolver.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ResultTypeResolver.kt index be5fca9788f..7780cfcc9bf 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ResultTypeResolver.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ResultTypeResolver.kt @@ -85,13 +85,67 @@ class ResultTypeResolver( } val subType = c.findSubType(variableWithConstraints) - // Super type should be the most flexible, sub type should be the least one - val superType = c.findSuperType(variableWithConstraints).makeFlexibleIfNecessary(c, variableWithConstraints.constraints) + val superType = c.findSuperType(variableWithConstraints) + + val similarCapturedTypesInK2 = with(c) { + isK2 && similarCapturedTypes(subType, superType) + } + + /** + * The special logic for handling similar captured types in K2 is needed because of the following. + * Currently, it's allowed to squash LOWER(type) and UPPER(type) constraints with the same type + * into one EQUALS(type) constraint. + * E.g. it's possible for constraints like LOWER(SomeCapturedType!) & UPPER(SomeCapturedTypes!). + * In such a situation [ResultTypeResolver] infers a relevant type variable into SomeCapturedType!, + * without any approximation. + * However, complex handling of DNN/non-DNN types sometimes lead to a situation (see e.g. KT-50134 example) + * when we have a pair of LOWER(SomeCapturedType&Any..SomeCapturedType?) and UPPER(SomeCapturedType!). + * These constraints use almost the same type, but they cannot be squashed. + * Moreover, [ResultTypeResolver] approximates captured from expression types when they came from UPPER or LOWER constraint. + * See [AbstractTypeApproximator.approximateToSuperType] and [AbstractTypeApproximator.approximateToSubType] calls below. + * As a result, [ResultTypeResolver] infers a relevant type variable to e.g. Any!, + * despite the fact the situation is almost the same as in case with matched constraints. + * This can produce unexpected NEW_INFERENCE_ERROR because other constraints are unmatched. + * + * To handle this situation, approximation of captured types for this case with similar constraints is disabled in K2. + */ + // TODO: consider skipping captured types approximation unconditionally (KT-66346) + val preparedSubType = when { + subType == null -> null + similarCapturedTypesInK2 -> subType + /** + * + * fun Array.intersect(other: Iterable) { + * val set = toMutableSet() + * set.retainAll(other) + * } + * fun Array.toMutableSet(): MutableSet = ... + * fun MutableCollection.retainAll(elements: Iterable) {} + * + * Here, when we solve type system for `toMutableSet` we have the following constrains: + * Array <: Array => C(out X) <: T. + * If we fix it to T = C(out X) then return type of `toMutableSet()` will be `MutableSet` + * and type of variable `set` will be `MutableSet` and the following line will have contradiction. + * + * To fix this problem when we fix variable, we will approximate captured types before fixation. + * + */ + else -> typeApproximator.approximateToSuperType(subType, TypeApproximatorConfiguration.InternalTypesApproximation) ?: subType + } + + // TODO: consider skipping captured types approximation unconditionally (KT-66346) + val preparedSuperType = when { + superType == null -> null + similarCapturedTypesInK2 -> superType + c.isK2 && c.hasRecursiveTypeParametersWithGivenSelfType(superType.typeConstructor(c)) -> superType + else -> typeApproximator.approximateToSubType(superType, TypeApproximatorConfiguration.InternalTypesApproximation) ?: superType + // Super type should be the most flexible, sub type should be the least one + }.makeFlexibleIfNecessary(c, variableWithConstraints.constraints) val resultTypeFromDirection = if (direction == ResolveDirection.TO_SUBTYPE || direction == ResolveDirection.UNKNOWN) { - c.resultType(subType, superType, variableWithConstraints) + c.resultType(preparedSubType, preparedSuperType, variableWithConstraints) } else { - c.resultType(superType, subType, variableWithConstraints) + c.resultType(preparedSuperType, preparedSubType, variableWithConstraints) } // In the general case, we can have here two types, one from EQUAL constraint which must be ILT-based, // and the second one from UPPER/LOWER constraints (subType/superType based) @@ -108,6 +162,22 @@ class ResultTypeResolver( } } + /** + * This function returns true for a pair of captured-type based types like + * CapturedType&Any..CapturedType? and CapturedType..CapturedType?. + * Type constructors of lower/upper bound of both types should be the same captured types, to get true result. + * This is needed to avoid captured types approximation in such situation. + */ + private fun Context.similarCapturedTypes(subType: KotlinTypeMarker?, superType: KotlinTypeMarker?): Boolean { + if (subType == null) return false + if (superType == null) return false + val typeConstructor = subType.lowerBoundIfFlexible().typeConstructor() + return typeConstructor.isCapturedTypeConstructor() && + typeConstructor == subType.upperBoundIfFlexible().typeConstructor() && + typeConstructor == superType.lowerBoundIfFlexible().typeConstructor() && + typeConstructor == superType.upperBoundIfFlexible().typeConstructor() + } + /* * We propagate nullness flexibility into the result type from type variables in other constraints * to prevent variable fixation into less flexible type. @@ -238,28 +308,7 @@ class ResultTypeResolver( } } - /** - * - * fun Array.intersect(other: Iterable) { - * val set = toMutableSet() - * set.retainAll(other) - * } - * fun Array.toMutableSet(): MutableSet = ... - * fun MutableCollection.retainAll(elements: Iterable) {} - * - * Here, when we solve type system for `toMutableSet` we have the following constrains: - * Array <: Array => C(out X) <: T. - * If we fix it to T = C(out X) then return type of `toMutableSet()` will be `MutableSet` - * and type of variable `set` will be `MutableSet` and the following line will have contradiction. - * - * To fix this problem when we fix variable, we will approximate captured types before fixation. - * - */ - - return typeApproximator.approximateToSuperType( - commonSuperType, - TypeApproximatorConfiguration.InternalTypesApproximation - ) ?: commonSuperType + return commonSuperType } return null @@ -348,16 +397,7 @@ class ResultTypeResolver( variableWithConstraints.constraints.filter { it.kind == ConstraintKind.UPPER && this@findSuperType.isProperTypeForFixation(it.type) } if (upperConstraints.isNotEmpty()) { - val upperType = computeUpperType(upperConstraints) - - if (isK2 && hasRecursiveTypeParametersWithGivenSelfType(upperType.typeConstructor())) { - return upperType - } - - return typeApproximator.approximateToSubType( - upperType, - TypeApproximatorConfiguration.InternalTypesApproximation - ) ?: upperType + return computeUpperType(upperConstraints) } return null diff --git a/compiler/testData/diagnostics/tests/j+k/withTestConnection.fir.kt b/compiler/testData/diagnostics/tests/j+k/withTestConnection.fir.kt deleted file mode 100644 index 1255138bd41..00000000000 --- a/compiler/testData/diagnostics/tests/j+k/withTestConnection.fir.kt +++ /dev/null @@ -1,35 +0,0 @@ -// FULL_JDK -// JVM_TARGET: 1.8 - -// FILE: test.kt - -fun waitTestConnection(configurable: A) { - val res = configurable.test() - C.wait(res!!.toFuture()) -} - -// FILE: A.java - -import org.jetbrains.annotations.*; - -public interface A { - @Nullable B test(); -} - -// FILE: B.kt - -import java.util.concurrent.CompletableFuture - -abstract class B { - abstract fun toFuture(): CompletableFuture -} - -// FILE: C.java - -import java.util.concurrent.Future; - -public class C { - public static F wait(Future future) { - return null; - } -} diff --git a/compiler/testData/diagnostics/tests/j+k/withTestConnection.kt b/compiler/testData/diagnostics/tests/j+k/withTestConnection.kt index 0cb03b067d4..196cef81c2d 100644 --- a/compiler/testData/diagnostics/tests/j+k/withTestConnection.kt +++ b/compiler/testData/diagnostics/tests/j+k/withTestConnection.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // FULL_JDK // JVM_TARGET: 1.8