From 9f7876efb61ba5ee3c068a39b21eb64d8f591330 Mon Sep 17 00:00:00 2001 From: "Denis.Zharkov" Date: Thu, 5 May 2022 16:05:08 +0300 Subject: [PATCH] FIR: Avoid potentially dangerous checks at makesSenseToBeDefinitelyNotNull Sometimes, it might be called before type parameter bounds are initialized or even before the symbols are bound to FIR In such cases, we just assume it makes sense to create DNN there --- .../kotlin/fir/java/JavaTypeConversion.kt | 5 +-- .../fir/resolve/substitution/Substitutors.kt | 6 ++-- .../jetbrains/kotlin/fir/types/TypeUtils.kt | 33 +++++++++++++------ .../inference/capturedTypes/kt46727.fir.kt | 2 +- .../capturedTypes/kt46727Warnings.fir.kt | 4 +-- .../writerAppenderExampleRecursive.fir.kt | 6 ++-- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaTypeConversion.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaTypeConversion.kt index a3ea4a58bc3..a6dc8107000 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaTypeConversion.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/JavaTypeConversion.kt @@ -7,15 +7,12 @@ package org.jetbrains.kotlin.fir.java import org.jetbrains.kotlin.builtins.jvm.JavaToKotlinClassMap import org.jetbrains.kotlin.fir.FirSession -import org.jetbrains.kotlin.fir.declarations.FirTypeParameter -import org.jetbrains.kotlin.fir.diagnostics.ConeIntermediateDiagnostic import org.jetbrains.kotlin.fir.diagnostics.ConeSimpleDiagnostic import org.jetbrains.kotlin.fir.diagnostics.DiagnosticKind import org.jetbrains.kotlin.fir.java.enhancement.readOnlyToMutable import org.jetbrains.kotlin.fir.resolve.toFirRegularClassSymbol import org.jetbrains.kotlin.fir.symbols.ConeClassLikeLookupTag import org.jetbrains.kotlin.fir.symbols.impl.ConeClassLikeLookupTagImpl -import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol import org.jetbrains.kotlin.fir.types.* import org.jetbrains.kotlin.fir.types.builder.buildResolvedTypeRef import org.jetbrains.kotlin.fir.types.impl.ConeTypeParameterTypeImpl @@ -109,7 +106,7 @@ private fun JavaType?.toConeTypeProjection( lowerBound, session.typeContext, // Upper bounds might be not initialized properly yet, so we force creating DefinitelyNotNullType // It should not affect semantics, since it would be still a valid type anyway - forceWithoutCheck = true, + avoidComprehensiveCheck = true, ) ?: lowerBound else -> lowerBound } diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/resolve/substitution/Substitutors.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/resolve/substitution/Substitutors.kt index 1a720384a2c..8666ce3957f 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/resolve/substitution/Substitutors.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/resolve/substitution/Substitutors.kt @@ -47,7 +47,7 @@ abstract class AbstractConeSubstitutor(protected val typeContext: ConeTypeContex override fun substituteOrNull(type: ConeKotlinType): ConeKotlinType? { val newType = substituteType(type) if (newType != null && type is ConeDefinitelyNotNullType) { - return newType.makeConeTypeDefinitelyNotNullOrNotNull(typeContext) + return newType.makeConeTypeDefinitelyNotNullOrNotNull(typeContext, avoidComprehensiveCheck = false) } return (newType ?: type.substituteRecursive()) } @@ -109,7 +109,9 @@ abstract class AbstractConeSubstitutor(protected val typeContext: ConeTypeContex typeContext, substitutedOriginal.attributes.add(original.attributes) ) - return ConeDefinitelyNotNullType.create(substituted, typeContext) ?: substituted + return ConeDefinitelyNotNullType.create( + substituted, typeContext, avoidComprehensiveCheck = true, + ) ?: substituted } private fun ConeFlexibleType.substituteBounds(): ConeFlexibleType? { diff --git a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/TypeUtils.kt b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/TypeUtils.kt index 13e6f7be42b..bc018dc63e5 100644 --- a/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/TypeUtils.kt +++ b/compiler/fir/providers/src/org/jetbrains/kotlin/fir/types/TypeUtils.kt @@ -58,13 +58,16 @@ fun ConeInferenceContext.intersectTypesOrNull(types: List): Cone fun TypeCheckerProviderContext.equalTypes(a: ConeKotlinType, b: ConeKotlinType): Boolean = AbstractTypeChecker.equalTypes(this, a, b) -private fun ConeTypeContext.makesSenseToBeDefinitelyNotNull(originalType: ConeKotlinType): Boolean { - return when (val type = originalType.lowerBoundIfFlexible()) { - is ConeTypeParameterType -> !type.lookupTag.symbol.isInitializedFir || type.isNullableType() +private fun ConeTypeContext.makesSenseToBeDefinitelyNotNull( + type: ConeSimpleKotlinType, + avoidComprehensiveCheck: Boolean, +): Boolean { + return when (type) { + is ConeTypeParameterType -> avoidComprehensiveCheck || type.isNullableType() // Actually, this branch should work for type parameters as well, but it breaks some cases. See KT-40114. // Basically, if we have `T : X..X?`, then `T <: Any` but we still have `T` != `T & Any`. is ConeTypeVariableType, is ConeCapturedType -> { - !AbstractNullabilityChecker.isSubtypeOfAny( + avoidComprehensiveCheck || !AbstractNullabilityChecker.isSubtypeOfAny( newTypeCheckerState(errorTypesEqualToAnything = false, stubTypesEqualToAnything = false), type ) } @@ -76,22 +79,32 @@ private fun ConeTypeContext.makesSenseToBeDefinitelyNotNull(originalType: ConeKo fun ConeDefinitelyNotNullType.Companion.create( original: ConeKotlinType, typeContext: ConeTypeContext, - forceWithoutCheck: Boolean = false, + // Sometimes, it might be called before type parameter bounds are initialized + // or even before the symbols are bound to FIR + // In such cases, we just assume it makes sense to create DNN there + // NB: `makesSenseToBeDefinitelyNotNull` is mostly an optimization, it should not affect semantics + avoidComprehensiveCheck: Boolean = false, ): ConeDefinitelyNotNullType? { return when (original) { is ConeDefinitelyNotNullType -> original - is ConeFlexibleType -> create(original.lowerBound, typeContext) - is ConeSimpleKotlinType -> runIf(forceWithoutCheck || typeContext.makesSenseToBeDefinitelyNotNull(original)) { + is ConeFlexibleType -> create(original.lowerBound, typeContext, avoidComprehensiveCheck) + is ConeSimpleKotlinType -> runIf(typeContext.makesSenseToBeDefinitelyNotNull(original, avoidComprehensiveCheck)) { ConeDefinitelyNotNullType(original.coneLowerBoundIfFlexible()) } } } -fun ConeKotlinType.makeConeTypeDefinitelyNotNullOrNotNull(typeContext: ConeTypeContext): ConeKotlinType { +fun ConeKotlinType.makeConeTypeDefinitelyNotNullOrNotNull( + typeContext: ConeTypeContext, + avoidComprehensiveCheck: Boolean = false, +): ConeKotlinType { if (this is ConeIntersectionType) { - return ConeIntersectionType(intersectedTypes.map { it.makeConeTypeDefinitelyNotNullOrNotNull(typeContext) }) + return ConeIntersectionType(intersectedTypes.map { + it.makeConeTypeDefinitelyNotNullOrNotNull(typeContext, avoidComprehensiveCheck) + }) } - return ConeDefinitelyNotNullType.create(this, typeContext) ?: this.withNullability(ConeNullability.NOT_NULL, typeContext) + return ConeDefinitelyNotNullType.create(this, typeContext, avoidComprehensiveCheck) + ?: this.withNullability(ConeNullability.NOT_NULL, typeContext) } fun T.withArguments(arguments: Array): T { diff --git a/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727.fir.kt b/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727.fir.kt index eda76f0ed30..407314031be 100644 --- a/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727.fir.kt +++ b/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727.fir.kt @@ -89,7 +89,7 @@ fun main4() { fun takeStarFoo3(x: Foo3<*>) { x.value = "test" - x.value += "test" + x.value += "test" } fun main5() { diff --git a/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727Warnings.fir.kt b/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727Warnings.fir.kt index fb81fc6d661..24d87366bde 100644 --- a/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727Warnings.fir.kt +++ b/compiler/testData/diagnostics/tests/inference/capturedTypes/kt46727Warnings.fir.kt @@ -89,7 +89,7 @@ fun main4() { fun takeStarFoo3(x: Foo3<*>) { x.value = "test" - x.value += "test" + x.value += "test" } fun main5() { @@ -115,4 +115,4 @@ fun main6() { bar.value = 1 takeStarBar3(bar) println(bar.value) // CCE: String cannot be cast to Number -} \ No newline at end of file +} diff --git a/compiler/testData/diagnostics/tests/inference/recursiveTypes/selfTypes/enabledInferenceOnSelfTypes/writerAppenderExampleRecursive.fir.kt b/compiler/testData/diagnostics/tests/inference/recursiveTypes/selfTypes/enabledInferenceOnSelfTypes/writerAppenderExampleRecursive.fir.kt index 905bc4e767f..a9ec8f95316 100644 --- a/compiler/testData/diagnostics/tests/inference/recursiveTypes/selfTypes/enabledInferenceOnSelfTypes/writerAppenderExampleRecursive.fir.kt +++ b/compiler/testData/diagnostics/tests/inference/recursiveTypes/selfTypes/enabledInferenceOnSelfTypes/writerAppenderExampleRecursive.fir.kt @@ -26,10 +26,10 @@ fun test() { } fun testJava(appender: JavaWriterAppender) { - ..JavaWriterAppender.Builder1<*>?!")!>appender.newBuilder() - ..JavaWriterAppender.Builder1<*>?!>")!>appender.Builder1() + ..JavaWriterAppender.Builder1<*>?!"), NEW_INFERENCE_ERROR!>appender.newBuilder() + ..JavaWriterAppender.Builder1<*>?!>"), NEW_INFERENCE_ERROR!>appender.Builder1() - ..JavaWriterAppender.Builder1<*>?! & JavaWriterAppender.Builder2<*>..JavaWriterAppender.Builder2<*>?!..JavaWriterAppender.Builder1<*>? & JavaWriterAppender.Builder2<*>?")!>appender.intersectTwoSelfTypes() + & JavaWriterAppender.Builder2<*>..JavaWriterAppender.Builder1<*>? & JavaWriterAppender.Builder2<*>?"), NEW_INFERENCE_ERROR!>appender.intersectTwoSelfTypes() } object WriterAppender {