From 953f6ba6b608cd3c82e74b87de2ee40cc2dbb8c5 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 7 Nov 2023 19:22:44 +0100 Subject: [PATCH] K2: keep FlexibleNullability on enhanced type parameter types Types based on type parameters which have both hasEnhancedNullability and hasFlexibleNullability were incorrectly converted to IR: only `@EnhancedNullability` annotation was added to the IR type. This means that the nullability of the type started to matter, which is incorrect because the type is supposed to be flexible. This backfired in the IR fake override builder mode, where nullability of methods inherited from both Kotlin maps and Java maps clashed, which resulted in the exception from FakeOverrideRebuilder. If the type is both flexible and enhanced, we're now adding `@FlexibleNullability` to the IR type. Note that `@EnhancedNullability` is not needed because it only affects primitive types which need to be handled as wrapper types on JVM, but `@FlexibleNullability` already has that effect. IR still slightly differs from K1 for collection subclasses, which can be seen for example in the changed IR text tests. This is to be investigated later. The change in tests is needed because the `kt48495*.kt` tests are now passing on K2 in the IR fake override builder mode, while failing on the vanilla K2. This is related to KT-61370 but does not entirely fix it, although curiously it fixes all remaining tests which were muted because of it. --- .../kotlin/fir/backend/Fir2IrTypeConverter.kt | 8 ++++---- .../mapRemove/noDefaultImpls.kt | 1 - .../testData/codegen/box/collections/kt48945.kt | 2 +- .../testData/codegen/box/collections/kt48945a.kt | 2 +- .../testData/codegen/box/collections/kt48945b.kt | 2 +- .../testData/codegen/box/collections/kt48945c.kt | 2 +- compiler/testData/codegen/box/jdk/removeIf.kt | 1 - .../firProblems/ArrayListOverrides.fir.ir.txt | 16 ++++++++-------- .../ir/irText/stubs/builtinMap.fir.ir.txt | 2 +- ...degenWithIrFakeOverrideGeneratorSuppressor.kt | 5 +++++ 10 files changed, 22 insertions(+), 19 deletions(-) diff --git a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrTypeConverter.kt b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrTypeConverter.kt index 343f3b08fed..46283914669 100644 --- a/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrTypeConverter.kt +++ b/compiler/fir/fir2ir/src/org/jetbrains/kotlin/fir/backend/Fir2IrTypeConverter.kt @@ -128,13 +128,13 @@ class Fir2IrTypeConverter( ?: return createErrorType() when { - hasEnhancedNullability -> { - builtIns.enhancedNullabilityAnnotationConstructorCall()?.let { + hasFlexibleNullability -> { + builtIns.flexibleNullabilityAnnotationConstructorCall()?.let { typeAnnotations += it } } - hasFlexibleNullability -> { - builtIns.flexibleNullabilityAnnotationConstructorCall()?.let { + hasEnhancedNullability -> { + builtIns.enhancedNullabilityAnnotationConstructorCall()?.let { typeAnnotations += it } } diff --git a/compiler/testData/codegen/box/builtinStubMethods/mapRemove/noDefaultImpls.kt b/compiler/testData/codegen/box/builtinStubMethods/mapRemove/noDefaultImpls.kt index 93b143ba18d..b7e3a4441b6 100644 --- a/compiler/testData/codegen/box/builtinStubMethods/mapRemove/noDefaultImpls.kt +++ b/compiler/testData/codegen/box/builtinStubMethods/mapRemove/noDefaultImpls.kt @@ -1,6 +1,5 @@ // !JVM_DEFAULT_MODE: all // SKIP_JDK6 -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61370 // TARGET_BACKEND: JVM // JVM_TARGET: 1.8 // WITH_STDLIB diff --git a/compiler/testData/codegen/box/collections/kt48945.kt b/compiler/testData/codegen/box/collections/kt48945.kt index c996a7099a7..5b03a92ac87 100644 --- a/compiler/testData/codegen/box/collections/kt48945.kt +++ b/compiler/testData/codegen/box/collections/kt48945.kt @@ -1,7 +1,7 @@ // TARGET_BACKEND: JVM // JVM_TARGET: 1.8 // IGNORE_BACKEND_K2: JVM_IR, JS_IR -// FIR status: in progress (M.G.), different structure of f/o +// FIR status: different structure of fake overrides. Fixed in the IR fake override builder. // IGNORE_BACKEND: ANDROID // ^ NSME: java.util.AbstractMap.remove // FULL_JDK diff --git a/compiler/testData/codegen/box/collections/kt48945a.kt b/compiler/testData/codegen/box/collections/kt48945a.kt index 8e505c3a3fa..c40e43afe28 100644 --- a/compiler/testData/codegen/box/collections/kt48945a.kt +++ b/compiler/testData/codegen/box/collections/kt48945a.kt @@ -1,7 +1,7 @@ // TARGET_BACKEND: JVM // JVM_TARGET: 1.8 // IGNORE_BACKEND_K2: JVM_IR, JS_IR -// FIR status: in progress (M.G.), different structure of f/o +// FIR status: different structure of fake overrides. Fixed in the IR fake override builder. // IGNORE_BACKEND: ANDROID // ^ NSME: java.util.AbstractMap.remove // FULL_JDK diff --git a/compiler/testData/codegen/box/collections/kt48945b.kt b/compiler/testData/codegen/box/collections/kt48945b.kt index bf8facd45ed..b4c6bf9f7ce 100644 --- a/compiler/testData/codegen/box/collections/kt48945b.kt +++ b/compiler/testData/codegen/box/collections/kt48945b.kt @@ -1,7 +1,7 @@ // TARGET_BACKEND: JVM // JVM_TARGET: 1.8 // IGNORE_BACKEND_K2: JVM_IR, JS_IR -// FIR status: in progress (M.G.), different structure of f/o +// FIR status: different structure of fake overrides. Fixed in the IR fake override builder. // IGNORE_BACKEND: ANDROID // ^ NSME: java.util.AbstractMap.remove // FULL_JDK diff --git a/compiler/testData/codegen/box/collections/kt48945c.kt b/compiler/testData/codegen/box/collections/kt48945c.kt index 70d981574e6..958c77a0df2 100644 --- a/compiler/testData/codegen/box/collections/kt48945c.kt +++ b/compiler/testData/codegen/box/collections/kt48945c.kt @@ -1,7 +1,7 @@ // TARGET_BACKEND: JVM // JVM_TARGET: 1.8 // IGNORE_BACKEND_K2: JVM_IR, JS_IR -// FIR status: in progress (M.G.), different structure of f/o +// FIR status: different structure of fake overrides. Fixed in the IR fake override builder. // IGNORE_BACKEND: ANDROID // ^ NSME: java.util.AbstractMap.remove // FULL_JDK diff --git a/compiler/testData/codegen/box/jdk/removeIf.kt b/compiler/testData/codegen/box/jdk/removeIf.kt index 77b1eb64200..9b77fcf3e6c 100644 --- a/compiler/testData/codegen/box/jdk/removeIf.kt +++ b/compiler/testData/codegen/box/jdk/removeIf.kt @@ -1,6 +1,5 @@ // SKIP_JDK6 // TARGET_BACKEND: JVM -// IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION: KT-61370 // FULL_JDK // WITH_STDLIB diff --git a/compiler/testData/ir/irText/firProblems/ArrayListOverrides.fir.ir.txt b/compiler/testData/ir/irText/firProblems/ArrayListOverrides.fir.ir.txt index c4a1b7933a5..9eb71776ed9 100644 --- a/compiler/testData/ir/irText/firProblems/ArrayListOverrides.fir.ir.txt +++ b/compiler/testData/ir/irText/firProblems/ArrayListOverrides.fir.ir.txt @@ -175,13 +175,13 @@ FILE fqName: fileName:/ArrayListOverrides.kt overridden: public open fun toString (): @[EnhancedNullability] kotlin.String declared in java.util.ArrayList $this: VALUE_PARAMETER name: type:java.util.AbstractCollection<@[FlexibleNullability] kotlin.String?> - FUN FAKE_OVERRIDE name:stream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] kotlin.String?> [fake_override] + FUN FAKE_OVERRIDE name:stream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] kotlin.String?> [fake_override] overridden: - public open fun stream (): @[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] E of java.util.ArrayList?> declared in java.util.ArrayList + public open fun stream (): @[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] E of java.util.ArrayList?> declared in java.util.ArrayList $this: VALUE_PARAMETER name: type:kotlin.collections.Collection - FUN FAKE_OVERRIDE name:parallelStream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] kotlin.String?> [fake_override] + FUN FAKE_OVERRIDE name:parallelStream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] kotlin.String?> [fake_override] overridden: - public open fun parallelStream (): @[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] E of java.util.ArrayList?> declared in java.util.ArrayList + public open fun parallelStream (): @[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] E of java.util.ArrayList?> declared in java.util.ArrayList $this: VALUE_PARAMETER name: type:kotlin.collections.Collection FUN FAKE_OVERRIDE name:removeAt visibility:public modality:OPEN <> ($this:java.util.ArrayList, p0:kotlin.Int) returnType:@[EnhancedNullability] kotlin.String [fake_override,operator] overridden: @@ -367,13 +367,13 @@ FILE fqName: fileName:/ArrayListOverrides.kt overridden: public open fun toString (): @[EnhancedNullability] kotlin.String declared in java.util.ArrayList $this: VALUE_PARAMETER name: type:java.util.AbstractCollection<@[FlexibleNullability] kotlin.String?> - FUN FAKE_OVERRIDE name:stream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] kotlin.String?> [fake_override] + FUN FAKE_OVERRIDE name:stream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] kotlin.String?> [fake_override] overridden: - public open fun stream (): @[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] E of java.util.ArrayList?> declared in java.util.ArrayList + public open fun stream (): @[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] E of java.util.ArrayList?> declared in java.util.ArrayList $this: VALUE_PARAMETER name: type:kotlin.collections.Collection - FUN FAKE_OVERRIDE name:parallelStream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] kotlin.String?> [fake_override] + FUN FAKE_OVERRIDE name:parallelStream visibility:public modality:OPEN <> ($this:kotlin.collections.Collection) returnType:@[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] kotlin.String?> [fake_override] overridden: - public open fun parallelStream (): @[EnhancedNullability] java.util.stream.Stream<@[EnhancedNullability] E of java.util.ArrayList?> declared in java.util.ArrayList + public open fun parallelStream (): @[EnhancedNullability] java.util.stream.Stream<@[FlexibleNullability] E of java.util.ArrayList?> declared in java.util.ArrayList $this: VALUE_PARAMETER name: type:kotlin.collections.Collection FUN FAKE_OVERRIDE name:removeAt visibility:public modality:OPEN <> ($this:java.util.ArrayList, p0:kotlin.Int) returnType:@[EnhancedNullability] kotlin.String [fake_override,operator] overridden: diff --git a/compiler/testData/ir/irText/stubs/builtinMap.fir.ir.txt b/compiler/testData/ir/irText/stubs/builtinMap.fir.ir.txt index d657efa75ce..56da0611b8a 100644 --- a/compiler/testData/ir/irText/stubs/builtinMap.fir.ir.txt +++ b/compiler/testData/ir/irText/stubs/builtinMap.fir.ir.txt @@ -27,7 +27,7 @@ FILE fqName: fileName:/builtinMap.kt $receiver: VALUE_PARAMETER name:$this$apply type:java.util.LinkedHashMap<@[FlexibleNullability] K1 of .plus?, @[FlexibleNullability] V1 of .plus?> BLOCK_BODY TYPE_OP type=kotlin.Unit origin=IMPLICIT_COERCION_TO_UNIT typeOperand=kotlin.Unit - CALL 'public open fun put (p0: @[EnhancedNullability] K of java.util.LinkedHashMap?, p1: @[EnhancedNullability] V of java.util.LinkedHashMap?): V of java.util.LinkedHashMap? declared in java.util.LinkedHashMap' type=V1 of .plus? origin=null + CALL 'public open fun put (p0: @[FlexibleNullability] K of java.util.LinkedHashMap?, p1: @[FlexibleNullability] V of java.util.LinkedHashMap?): V of java.util.LinkedHashMap? declared in java.util.LinkedHashMap' type=V1 of .plus? origin=null $this: GET_VAR '$this$apply: java.util.LinkedHashMap<@[FlexibleNullability] K1 of .plus?, @[FlexibleNullability] V1 of .plus?> declared in .plus.' type=java.util.LinkedHashMap<@[FlexibleNullability] K1 of .plus?, @[FlexibleNullability] V1 of .plus?> origin=null p0: CALL 'public final fun (): A of kotlin.Pair declared in kotlin.Pair' type=K1 of .plus origin=GET_PROPERTY $this: GET_VAR 'pair: kotlin.Pair.plus, V1 of .plus> declared in .plus' type=kotlin.Pair.plus, V1 of .plus> origin=null diff --git a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/backend/ir/CodegenWithIrFakeOverrideGeneratorSuppressor.kt b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/backend/ir/CodegenWithIrFakeOverrideGeneratorSuppressor.kt index 0b56ab7ba6f..3c59b45ef77 100644 --- a/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/backend/ir/CodegenWithIrFakeOverrideGeneratorSuppressor.kt +++ b/compiler/tests-common-new/tests/org/jetbrains/kotlin/test/backend/ir/CodegenWithIrFakeOverrideGeneratorSuppressor.kt @@ -6,10 +6,12 @@ package org.jetbrains.kotlin.test.backend.ir import org.jetbrains.kotlin.test.WrappedException +import org.jetbrains.kotlin.test.directives.CodegenTestDirectives import org.jetbrains.kotlin.test.directives.CodegenTestDirectives.ENABLE_IR_FAKE_OVERRIDE_GENERATION import org.jetbrains.kotlin.test.directives.CodegenTestDirectives.IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION import org.jetbrains.kotlin.test.model.AfterAnalysisChecker import org.jetbrains.kotlin.test.services.TestServices +import org.jetbrains.kotlin.test.services.defaultsProvider import org.jetbrains.kotlin.test.services.moduleStructure class CodegenWithIrFakeOverrideGeneratorSuppressor(testServices: TestServices) : AfterAnalysisChecker(testServices) { @@ -22,6 +24,9 @@ class CodegenWithIrFakeOverrideGeneratorSuppressor(testServices: TestServices) : AssertionError("Looks like this test can be unmuted. Remove $IGNORE_CODEGEN_WITH_IR_FAKE_OVERRIDE_GENERATION directive.").wrap() ) else emptyList() + testServices.defaultsProvider.defaultTargetBackend + in testServices.moduleStructure.allDirectives[CodegenTestDirectives.IGNORE_BACKEND_K2] -> + emptyList() else -> failedAssertions } }