From 560c1cacf3de3d1bfb75eceadf7cd7190d25b2a9 Mon Sep 17 00:00:00 2001 From: Kirill Rakhman Date: Tue, 21 Nov 2023 16:51:50 +0100 Subject: [PATCH] [FIR] Fix capturing of flexible types during resolution Previously, because we didn't handle flexible types properly in prepareCapturedType, projections inside flexible types would only be captured during subtyping with captureStatus=FOR_SUBTYPING which would lead to the constraint type being wrongly approximated (see ConstraintInjector.TypeCheckerStateForConstraintInjector .addNewIncorporatedConstraint). Fixing the capturing produced two kinds of false positive diagnostics: 1. In ConstraintInjector.TypeCheckerStateForConstraintInjector .addNewIncorporatedConstraint we would get two instances of cone types that are structurally equal and containing the same captured type. However, because we only skipped subtyping if the types were referentially equal, we would get a contradiction here. The fix was to use structural equality instead, which should be okay as the captured type instances are the same. 2. Reified type variables were inferred to captured types because flexible arrays with captured upper bounds (Array..Array?) were not properly approximated. #KT-62609 Fixed --- ...CompilerTestFE10TestdataTestGenerated.java | 6 +++ ...sticCompilerFE10TestDataTestGenerated.java | 6 +++ ...eeOldFrontendDiagnosticsTestGenerated.java | 6 +++ ...siOldFrontendDiagnosticsTestGenerated.java | 6 +++ .../kotlin/fir/types/ConeTypeContext.kt | 4 ++ .../kotlin/fir/resolve/calls/Arguments.kt | 26 ++-------- .../kotlin/ir/types/IrTypeSystemContext.kt | 3 ++ .../components/ConstraintInjector.kt | 7 ++- .../kotlin/types/AbstractTypeApproximator.kt | 27 ++++++++-- .../nonFixedVariableFromBothBranches.fir.kt | 12 ++++- .../nonFixedVariableFromBothBranches.kt | 12 ++++- .../nonFixedVariableFromBothBranches.txt | 15 ------ .../diagnostics/tests/inference/kt62609.kt | 52 +++++++++++++++++++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ .../kotlin/types/model/TypeSystemContext.kt | 1 + .../types/checker/ClassicTypeSystemContext.kt | 5 ++ 16 files changed, 148 insertions(+), 46 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.txt create mode 100644 compiler/testData/diagnostics/tests/inference/kt62609.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java index f055e2488ac..0386ccfe676 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java @@ -16424,6 +16424,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia runTest("compiler/testData/diagnostics/tests/inference/kt619.kt"); } + @Test + @TestMetadata("kt62609.kt") + public void testKt62609() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/kt62609.kt"); + } + @Test @TestMetadata("lambdaArgumentWithLabel.kt") public void testLambdaArgumentWithLabel() throws Exception { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java index 6cc71d824c9..d47371d1d6c 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java @@ -16424,6 +16424,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated runTest("compiler/testData/diagnostics/tests/inference/kt619.kt"); } + @Test + @TestMetadata("kt62609.kt") + public void testKt62609() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/kt62609.kt"); + } + @Test @TestMetadata("lambdaArgumentWithLabel.kt") public void testLambdaArgumentWithLabel() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index 2256af3383f..31a5b54212a 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -16418,6 +16418,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir runTest("compiler/testData/diagnostics/tests/inference/kt619.kt"); } + @Test + @TestMetadata("kt62609.kt") + public void testKt62609() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/kt62609.kt"); + } + @Test @TestMetadata("lambdaArgumentWithLabel.kt") public void testLambdaArgumentWithLabel() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index d36feace7a6..88bb14b69b2 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -16424,6 +16424,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia runTest("compiler/testData/diagnostics/tests/inference/kt619.kt"); } + @Test + @TestMetadata("kt62609.kt") + public void testKt62609() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/kt62609.kt"); + } + @Test @TestMetadata("lambdaArgumentWithLabel.kt") public void testLambdaArgumentWithLabel() throws Exception { diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/ConeTypeContext.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/ConeTypeContext.kt index e037cf6af89..57843d86c89 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/ConeTypeContext.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/ConeTypeContext.kt @@ -412,6 +412,10 @@ interface ConeTypeContext : TypeSystemContext, TypeSystemOptimizationContext, Ty return this is ConeClassLikeLookupTag && classId == StandardClassIds.Nothing } + override fun TypeConstructorMarker.isArrayConstructor(): Boolean { + return this is ConeClassLikeLookupTag && classId == StandardClassIds.Array + } + override fun SimpleTypeMarker.isSingleClassifierType(): Boolean { if (isError()) return false if (this is ConeCapturedType) return true diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt index 62b0beb630a..cff3f46b6e7 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/calls/Arguments.kt @@ -33,7 +33,6 @@ import org.jetbrains.kotlin.resolve.calls.inference.addSubtypeConstraintIfCompat import org.jetbrains.kotlin.resolve.calls.inference.model.ConstraintPosition import org.jetbrains.kotlin.resolve.calls.inference.model.SimpleConstraintSystemConstraintPosition import org.jetbrains.kotlin.types.AbstractTypeChecker -import org.jetbrains.kotlin.types.model.CaptureStatus import org.jetbrains.kotlin.types.model.TypeSystemCommonSuperTypesContext import org.jetbrains.kotlin.types.model.typeConstructor @@ -303,28 +302,9 @@ private fun argumentTypeWithCustomConversion( } } -fun Candidate.prepareCapturedType(argumentType: ConeKotlinType, context: ResolutionContext): ConeKotlinType { - return captureTypeFromExpressionOrNull(argumentType, context) ?: argumentType -} - -private fun Candidate.captureTypeFromExpressionOrNull(argumentType: ConeKotlinType, context: ResolutionContext): ConeKotlinType? { - val type = argumentType.fullyExpandedType(context.session) - if (type is ConeIntersectionType) { - val intersectedTypes = type.intersectedTypes.map { captureTypeFromExpressionOrNull(it, context) ?: it } - if (intersectedTypes == type.intersectedTypes) return null - return ConeIntersectionType( - intersectedTypes, - type.alternativeType?.let { captureTypeFromExpressionOrNull(it, context) ?: it } - ) - } - - if (type !is ConeClassLikeType && type !is ConeFlexibleType) return null - - if (type.typeArguments.isEmpty()) return null - - return context.session.typeContext.captureFromArgumentsInternal( - type, CaptureStatus.FROM_EXPRESSION - ) +internal fun prepareCapturedType(argumentType: ConeKotlinType, context: ResolutionContext): ConeKotlinType { + if (argumentType.isRaw()) return argumentType + return context.typeContext.captureFromExpression(argumentType.fullyExpandedType(context.session)) ?: argumentType } private fun checkApplicabilityForArgumentType( diff --git a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/types/IrTypeSystemContext.kt b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/types/IrTypeSystemContext.kt index d38e1408bab..bcd38662850 100644 --- a/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/types/IrTypeSystemContext.kt +++ b/compiler/ir/ir.tree/src/org/jetbrains/kotlin/ir/types/IrTypeSystemContext.kt @@ -303,6 +303,9 @@ interface IrTypeSystemContext : TypeSystemContext, TypeSystemCommonSuperTypesCon override fun TypeConstructorMarker.isNothingConstructor(): Boolean = this is IrClassSymbol && isClassWithFqName(StandardNames.FqNames.nothing) + override fun TypeConstructorMarker.isArrayConstructor(): Boolean = + this is IrClassSymbol && isClassWithFqName(StandardNames.FqNames.array) + override fun SimpleTypeMarker.isSingleClassifierType() = true override fun SimpleTypeMarker.possibleIntegerTypes() = irBuiltIns.run { diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ConstraintInjector.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ConstraintInjector.kt index b0d01c2a5b0..7fd39cb62e6 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ConstraintInjector.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/resolve/calls/inference/components/ConstraintInjector.kt @@ -466,7 +466,12 @@ class ConstraintInjector( isFromNullabilityConstraint: Boolean, isFromDeclaredUpperBound: Boolean ) { - if (lowerType === upperType) return + // Avoid checking trivial incorporated constraints + if (isK2) { + if (lowerType == upperType) return + } else { + if (lowerType === upperType) return + } if (c.isAllowedType(lowerType) && c.isAllowedType(upperType)) { fun runIsSubtypeOf() = runIsSubtypeOf(lowerType, upperType, shouldTryUseDifferentFlexibilityForUpperType, isFromNullabilityConstraint) diff --git a/compiler/resolution.common/src/org/jetbrains/kotlin/types/AbstractTypeApproximator.kt b/compiler/resolution.common/src/org/jetbrains/kotlin/types/AbstractTypeApproximator.kt index 09f54c4c144..c2c275d6bd3 100644 --- a/compiler/resolution.common/src/org/jetbrains/kotlin/types/AbstractTypeApproximator.kt +++ b/compiler/resolution.common/src/org/jetbrains/kotlin/types/AbstractTypeApproximator.kt @@ -143,10 +143,12 @@ abstract class AbstractTypeApproximator( val lowerResult = approximateTo(lowerBound, conf, depth) - val upperResult = if (!type.isRawType() && lowerBound.typeConstructor() == upperBound.typeConstructor()) + val upperResult = if (!type.isRawType() && !shouldApproximateUpperBoundSeparately(lowerBound, upperBound, conf)) { + // We skip approximating the upper bound if the type constructors match as an optimization. lowerResult?.withNullability(upperBound.isMarkedNullable()) - else + } else { approximateTo(upperBound, conf, depth) + } if (lowerResult == null && upperResult == null) return null /** @@ -168,11 +170,30 @@ abstract class AbstractTypeApproximator( } } + private fun shouldApproximateUpperBoundSeparately( + lowerBound: SimpleTypeMarker, + upperBound: SimpleTypeMarker, + conf: TypeApproximatorConfiguration, + ): Boolean { + val upperBoundConstructor = upperBound.typeConstructor() + if (lowerBound.typeConstructor() != upperBoundConstructor) return true + + // Flexible arrays have the shape `Array..Array?`. + // When such a type is captured, it results in `Array..Array?`, therefore it's necessary to approximate the + // upper bound separately. + // As an important performance optimization, we explicitly check if the type in question is an array with a captured type argument + // that needs to be approximated. + // This saves us from doing twice the work unnecessarily in many cases. + return isK2 && + upperBoundConstructor.isArrayConstructor() && + upperBound.getArgumentOrNull(0).let { it is CapturedTypeMarker && conf.capturedType(ctx, it) } + } + private fun approximateLocalTypes( type: SimpleTypeMarker, conf: TypeApproximatorConfiguration, toSuper: Boolean, - depth: Int + depth: Int, ): SimpleTypeMarker? { if (!toSuper) return null if (!conf.localTypes && !conf.anonymous) return null diff --git a/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.fir.kt b/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.fir.kt index e2dd3d86cb4..992cd2e3d9c 100644 --- a/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.fir.kt +++ b/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.fir.kt @@ -12,8 +12,16 @@ fun select(x: K, y: K): K = x fun foo(f: () -> R): R = f() +interface Inv { + fun createArray(): Array +} + fun test(n: Number) { val a = select(foo { JavaTest.createNumberArray() }, emptyArray()) + ..kotlin.Array?!")!>a +} - a -} \ No newline at end of file +fun test2(inv: Inv) { + val a = select(foo { inv.createArray() }, emptyArray()) + ")!>a +} diff --git a/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.kt b/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.kt index 75522981860..0ea1c625b6a 100644 --- a/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.kt +++ b/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.kt @@ -12,8 +12,16 @@ fun select(x: K, y: K): K = x fun foo(f: () -> R): R = f() +interface Inv { + fun createArray(): Array +} + fun test(n: Number) { val a = select(foo { JavaTest.createNumberArray() }, emptyArray()) - ..kotlin.Array?)")!>a -} \ No newline at end of file +} + +fun test2(inv: Inv) { + val a = select(foo { inv.createArray() }, emptyArray()) + ")!>a +} diff --git a/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.txt b/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.txt deleted file mode 100644 index f301afe0b13..00000000000 --- a/compiler/testData/diagnostics/tests/inference/commonSystem/nonFixedVariableFromBothBranches.txt +++ /dev/null @@ -1,15 +0,0 @@ -package - -public fun foo(/*0*/ f: () -> R): R -public fun select(/*0*/ x: K, /*1*/ y: K): K -public fun test(/*0*/ n: kotlin.Number): kotlin.Unit - -public open class JavaTest { - public constructor JavaTest() - public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int - public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String - - // Static members - public open fun createNumberArray(): kotlin.Array<(out) kotlin.Number!>! -} diff --git a/compiler/testData/diagnostics/tests/inference/kt62609.kt b/compiler/testData/diagnostics/tests/inference/kt62609.kt new file mode 100644 index 00000000000..36ef8638a3b --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/kt62609.kt @@ -0,0 +1,52 @@ +// FIR_IDENTICAL +// ISSUE: KT-62609 +// FILE: I.java +public interface I {} + +// FILE: X.java +public abstract class X

{} + +// FILE: A.java +public final class A extends X implements I { + public static final A INSTANCE = new A(); +} + +// FILE: B.java +public final class B extends X implements I { + public static final B INSTANCE = new B(); +} + +// FILE: test.kt +fun test1() { + controlFun(A.INSTANCE) + controlFun(B.INSTANCE) + controlFun2(A.INSTANCE) + controlFun2(B.INSTANCE) + + val a = when { + true -> A.INSTANCE + else -> B.INSTANCE + } + + controlFun(a) + controlFun2(a) +} + +fun test2() { + controlFun(A()) + controlFun(B()) + controlFun2(A()) + controlFun2(B()) + + val a = when { + true -> A() + else -> B() + } + + controlFun(a) + controlFun2(a) +} + + +fun controlFun(c: I) {} +fun controlFun2(c: X) {} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index 02822b02990..8672fb31933 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -16424,6 +16424,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/inference/kt619.kt"); } + @Test + @TestMetadata("kt62609.kt") + public void testKt62609() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/kt62609.kt"); + } + @Test @TestMetadata("lambdaArgumentWithLabel.kt") public void testLambdaArgumentWithLabel() throws Exception { diff --git a/core/compiler.common/src/org/jetbrains/kotlin/types/model/TypeSystemContext.kt b/core/compiler.common/src/org/jetbrains/kotlin/types/model/TypeSystemContext.kt index afc9816b985..ded923ce2a9 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/types/model/TypeSystemContext.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/types/model/TypeSystemContext.kt @@ -518,6 +518,7 @@ interface TypeSystemContext : TypeSystemOptimizationContext { fun TypeConstructorMarker.isAnyConstructor(): Boolean fun TypeConstructorMarker.isNothingConstructor(): Boolean + fun TypeConstructorMarker.isArrayConstructor(): Boolean /** * diff --git a/core/descriptors/src/org/jetbrains/kotlin/types/checker/ClassicTypeSystemContext.kt b/core/descriptors/src/org/jetbrains/kotlin/types/checker/ClassicTypeSystemContext.kt index 6518f370637..b79951b8b67 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/types/checker/ClassicTypeSystemContext.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/types/checker/ClassicTypeSystemContext.kt @@ -350,6 +350,11 @@ interface ClassicTypeSystemContext : TypeSystemInferenceExtensionContext, TypeSy return KotlinBuiltIns.isTypeConstructorForGivenClass(this, FqNames.nothing) } + override fun TypeConstructorMarker.isArrayConstructor(): Boolean { + require(this is TypeConstructor, this::errorMessage) + return KotlinBuiltIns.isTypeConstructorForGivenClass(this, FqNames.array) + } + override fun KotlinTypeMarker.asTypeArgument(): TypeArgumentMarker { require(this is KotlinType, this::errorMessage) return this.asTypeProjection()