From 5e4f022c596fbc00909d1131fa04c041d346a8f6 Mon Sep 17 00:00:00 2001 From: pyos Date: Mon, 30 Aug 2021 12:27:40 +0200 Subject: [PATCH] JVM_IR: use nullability when boxing/unboxing inline class types Given inline class V(Any?), a coercion from (Object, V) to (Object, V?) is boxing. In theory, the same issue in the old backend can be fixed by making `KotlinTypeMapper.mapUnderlyingTypeOfInlineClassType` use `computeExpandedTypeForInlineClass`, but for some reason this breaks a lot of stuff. #KT-48430 Fixed --- .../jetbrains/kotlin/codegen/StackValue.java | 42 ++++++++++-------- .../FirBlackBoxCodegenTestGenerated.java | 6 +++ .../backend/jvm/codegen/PromisedValue.kt | 43 ++++++++----------- .../boxNullableForFakeOverride.kt | 10 +++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 +++ .../IrBlackBoxCodegenTestGenerated.java | 6 +++ .../LightAnalysisModeTestGenerated.java | 5 +++ .../IrJsCodegenBoxES6TestGenerated.java | 5 +++ .../IrJsCodegenBoxTestGenerated.java | 5 +++ .../semantics/JsCodegenBoxTestGenerated.java | 5 +++ .../IrCodegenBoxWasmTestGenerated.java | 5 +++ 11 files changed, 95 insertions(+), 43 deletions(-) create mode 100644 compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java index 52c96a80739..4ac2fd6c0e0 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/StackValue.java @@ -458,14 +458,19 @@ public abstract class StackValue { public static void boxInlineClass( @NotNull KotlinTypeMarker kotlinType, @NotNull InstructionAdapter v, @NotNull KotlinTypeMapperBase typeMapper ) { - Type boxedType = typeMapper.mapTypeCommon(kotlinType, TypeMappingMode.CLASS_DECLARATION); - Type underlyingType = KotlinTypeMapper.mapUnderlyingTypeOfInlineClassType(kotlinType, typeMapper); + Type boxed = typeMapper.mapTypeCommon(kotlinType, TypeMappingMode.CLASS_DECLARATION); + Type unboxed = KotlinTypeMapper.mapUnderlyingTypeOfInlineClassType(kotlinType, typeMapper); + boolean isNullable = typeMapper.getTypeSystem().isNullableType(kotlinType) && !isPrimitive(unboxed); + boxInlineClass(unboxed, boxed, isNullable, v); + } - if (typeMapper.getTypeSystem().isNullableType(kotlinType) && !isPrimitive(underlyingType)) { - boxOrUnboxWithNullCheck(v, vv -> invokeBoxMethod(vv, boxedType, underlyingType)); - } - else { - invokeBoxMethod(v, boxedType, underlyingType); + public static void boxInlineClass( + @NotNull Type unboxed, @NotNull Type boxed, boolean isNullable, @NotNull InstructionAdapter v + ) { + if (isNullable) { + boxOrUnboxWithNullCheck(v, vv -> invokeBoxMethod(vv, boxed, unboxed)); + } else { + invokeBoxMethod(v, boxed, unboxed); } } @@ -488,17 +493,20 @@ public abstract class StackValue { @NotNull InstructionAdapter v, @NotNull KotlinTypeMapperBase typeMapper ) { - Type owner = typeMapper.mapTypeCommon(targetInlineClassType, TypeMappingMode.CLASS_DECLARATION); + Type boxed = typeMapper.mapTypeCommon(targetInlineClassType, TypeMappingMode.CLASS_DECLARATION); + Type unboxed = KotlinTypeMapper.mapUnderlyingTypeOfInlineClassType(targetInlineClassType, typeMapper); + boolean isNullable = typeMapper.getTypeSystem().isNullableType(targetInlineClassType) && !isPrimitive(unboxed); + unboxInlineClass(type, boxed, unboxed, isNullable, v); + } - coerce(type, owner, v); - - Type resultType = KotlinTypeMapper.mapUnderlyingTypeOfInlineClassType(targetInlineClassType, typeMapper); - - if (typeMapper.getTypeSystem().isNullableType(targetInlineClassType) && !isPrimitive(resultType)) { - boxOrUnboxWithNullCheck(v, vv -> invokeUnboxMethod(vv, owner, resultType)); - } - else { - invokeUnboxMethod(v, owner, resultType); + public static void unboxInlineClass( + @NotNull Type type, @NotNull Type boxed, @NotNull Type unboxed, boolean isNullable, @NotNull InstructionAdapter v + ) { + coerce(type, boxed, v); + if (isNullable) { + boxOrUnboxWithNullCheck(v, vv -> invokeUnboxMethod(vv, boxed, unboxed)); + } else { + invokeUnboxMethod(v, boxed, unboxed); } } diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java index 56445a73da3..97638c0dad5 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java @@ -18370,6 +18370,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @Test + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @Test @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/PromisedValue.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/PromisedValue.kt index ddcfcc810a3..7bb3e50b158 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/PromisedValue.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/codegen/PromisedValue.kt @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol import org.jetbrains.kotlin.ir.types.* import org.jetbrains.kotlin.ir.util.isTypeParameter import org.jetbrains.kotlin.ir.util.parentAsClass +import org.jetbrains.kotlin.load.kotlin.TypeMappingMode import org.jetbrains.kotlin.resolve.jvm.AsmTypes import org.jetbrains.org.objectweb.asm.Label import org.jetbrains.org.objectweb.asm.Type @@ -27,35 +28,25 @@ abstract class PromisedValue(val codegen: ExpressionCodegen, val type: Type, val open fun materializeAt(target: Type, irTarget: IrType, castForReified: Boolean) { val erasedSourceType = irType.eraseTypeParameters() val erasedTargetType = irTarget.eraseTypeParameters() - val fromTypeRepresentation = erasedSourceType.getClass()!!.inlineClassRepresentation - val toTypeRepresentation = erasedTargetType.getClass()!!.inlineClassRepresentation // Coerce inline classes - if (fromTypeRepresentation != null || toTypeRepresentation != null) { - val isFromTypeUnboxed = fromTypeRepresentation?.underlyingType?.let(typeMapper::mapType) == type - val isToTypeUnboxed = toTypeRepresentation?.underlyingType?.let(typeMapper::mapType) == target - - when { - isFromTypeUnboxed && !isToTypeUnboxed -> { - StackValue.boxInlineClass(erasedSourceType, mv, typeMapper) - return - } - - !isFromTypeUnboxed && isToTypeUnboxed -> { - val irClass = codegen.irFunction.parentAsClass - if (irClass.isInline && irClass.symbol == irType.classifierOrNull && !irType.isNullable()) { - // Use getfield instead of unbox-impl inside inline classes - codegen.mv.getfield( - typeMapper.classInternalName(irClass), - irClass.inlineClassFieldName.asString(), - typeMapper.mapType(irType).descriptor - ) - } else { - StackValue.unboxInlineClass(type, erasedTargetType, mv, typeMapper) - } - return - } + val isFromTypeUnboxed = InlineClassAbi.unboxType(erasedSourceType)?.let(typeMapper::mapType) == type + val isToTypeUnboxed = InlineClassAbi.unboxType(erasedTargetType)?.let(typeMapper::mapType) == target + if (isFromTypeUnboxed && !isToTypeUnboxed) { + val boxed = typeMapper.mapType(erasedSourceType, TypeMappingMode.CLASS_DECLARATION) + StackValue.boxInlineClass(type, boxed, erasedSourceType.isNullable(), mv) + return + } + if (!isFromTypeUnboxed && isToTypeUnboxed) { + val boxed = typeMapper.mapType(erasedTargetType, TypeMappingMode.CLASS_DECLARATION) + val irClass = codegen.irFunction.parentAsClass + if (irClass.isInline && irClass.symbol == irType.classifierOrNull && !irType.isNullable()) { + // Use getfield instead of unbox-impl inside inline classes + codegen.mv.getfield(boxed.internalName, irClass.inlineClassFieldName.asString(), target.descriptor) + } else { + StackValue.unboxInlineClass(type, boxed, target, erasedTargetType.isNullable(), mv) } + return } if (type != target || (castForReified && irType.anyTypeArgument { it.isReified })) { diff --git a/compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt b/compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt new file mode 100644 index 00000000000..e1b5b4b082c --- /dev/null +++ b/compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt @@ -0,0 +1,10 @@ +// IGNORE_BACKEND: JVM +abstract class C { + fun foo(v: T?, x: (T) -> Any?) = v?.let { x(it) } +} + +inline class V(val value: Any?) + +class D : C() + +fun box() = D().foo(V("OK")) { it.value } as String diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 860ab04f97b..35bf33961cc 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -18220,6 +18220,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @Test + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @Test @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index fcdb239ef96..ee91a35d73f 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -18370,6 +18370,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @Test + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @Test @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 6c8452e0d07..1e48833f1a6 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -15162,6 +15162,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { runTest("compiler/testData/codegen/box/inlineClasses/boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java index 197a6b4ff93..b98a176dbd7 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/es6/semantics/IrJsCodegenBoxES6TestGenerated.java @@ -13176,6 +13176,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { runTest("compiler/testData/codegen/box/inlineClasses/boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java index 6ef16b2ac90..dc5fb1ddf8c 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/semantics/IrJsCodegenBoxTestGenerated.java @@ -12582,6 +12582,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest { runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { runTest("compiler/testData/codegen/box/inlineClasses/boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 77b66474e2b..106a698a543 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -12647,6 +12647,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { runTest("compiler/testData/codegen/box/inlineClasses/boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt"); diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java index 6d87d8a3c77..80e0c1b3bb4 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/wasm/semantics/IrCodegenBoxWasmTestGenerated.java @@ -6727,6 +6727,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest runTest("compiler/testData/codegen/box/inlineClasses/boxImplDoesNotExecuteInitBlock.kt"); } + @TestMetadata("boxNullableForFakeOverride.kt") + public void testBoxNullableForFakeOverride() throws Exception { + runTest("compiler/testData/codegen/box/inlineClasses/boxNullableForFakeOverride.kt"); + } + @TestMetadata("boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt") public void testBoxNullableValueOfInlineClassWithNonNullUnderlyingType() throws Exception { runTest("compiler/testData/codegen/box/inlineClasses/boxNullableValueOfInlineClassWithNonNullUnderlyingType.kt");