From 868d0acf6bd5e550ae84428a6b62f9f6e1c2c633 Mon Sep 17 00:00:00 2001 From: Andrei Shikov Date: Wed, 1 May 2024 19:55:22 +0000 Subject: [PATCH] Ensure that inline body is realized when source information is off aosp/3032485 realized groups inside inline functions only when source information is enabled, which is incorrect. Test: compiler tests Fixes: 338179884 ( https://issuetracker.google.com/issues/338179884 ) Change-Id: Idddb882db6bf455032b12cf8a5a0d7d2bac85568 ( https://android-review.googlesource.com/q/Idddb882db6bf455032b12cf8a5a0d7d2bac85568 ) Moved from: https://github.com/androidx/androidx/commit/428fff56aa47ecf77cd686e1e6a7bda31f05b6b0 --- .../FunctionBodySkippingTransformTests.kt | 23 ++++++++++ ...mposableInlineFunction[useFir = false].txt | 43 +++++++++++++++++++ ...omposableInlineFunction[useFir = true].txt | 43 +++++++++++++++++++ .../ComposableFunctionBodyTransformer.kt | 12 +++--- 4 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = false].txt create mode 100644 plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = true].txt diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt index eb8198d833fdf..4026f598cd5b4 100644 --- a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt +++ b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt @@ -1409,4 +1409,27 @@ class FunctionBodySkippingTransformTestsNoSource( annotation class Type """ ) + + @Test + fun testInlineCallInsideComposableInlineFunction() = verifyGoldenComposeIrTransform( + source = """ + import androidx.compose.runtime.* + import androidx.compose.foundation.layout.* + + @Composable + fun Test(count: Int) { + Row { + repeat(count) { + Text("A") + } + } + } + """, + extra = """ + import androidx.compose.runtime.* + + @Composable + fun Text(value: String) {} + """ + ) } diff --git a/plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = false].txt b/plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = false].txt new file mode 100644 index 0000000000000..66da06f62e484 --- /dev/null +++ b/plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = false].txt @@ -0,0 +1,43 @@ +// +// Source +// ------------------------------------------ + +import androidx.compose.runtime.* +import androidx.compose.foundation.layout.* + +@Composable +fun Test(count: Int) { + Row { + repeat(count) { + Text("A") + } + } +} + +// +// Transformed IR +// ------------------------------------------ + +@Composable +@ComposableTarget(applier = "androidx.compose.ui.UiComposable") +fun Test(count: Int, %composer: Composer?, %changed: Int) { + %composer = %composer.startRestartGroup(<>) + val %dirty = %changed + if (%changed and 0b1110 == 0) { + %dirty = %dirty or if (%composer.changed(count)) 0b0100 else 0b0010 + } + if (%dirty and 0b1011 != 0b0010 || !%composer.skipping) { + Row(null, null, null, { %composer: Composer?, %changed: Int -> + %composer.startReplaceGroup(<>) + repeat(count) { it: Int -> + Text("A", %composer, 0b0110) + } + %composer.endReplaceGroup() + }, %composer, 0, 0b0111) + } else { + %composer.skipToGroupEnd() + } + %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int -> + Test(count, %composer, updateChangedFlags(%changed or 0b0001)) + } +} diff --git a/plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = true].txt b/plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = true].txt new file mode 100644 index 0000000000000..66da06f62e484 --- /dev/null +++ b/plugins/compose/compiler-hosted/integration-tests/src/test/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/testInlineCallInsideComposableInlineFunction[useFir = true].txt @@ -0,0 +1,43 @@ +// +// Source +// ------------------------------------------ + +import androidx.compose.runtime.* +import androidx.compose.foundation.layout.* + +@Composable +fun Test(count: Int) { + Row { + repeat(count) { + Text("A") + } + } +} + +// +// Transformed IR +// ------------------------------------------ + +@Composable +@ComposableTarget(applier = "androidx.compose.ui.UiComposable") +fun Test(count: Int, %composer: Composer?, %changed: Int) { + %composer = %composer.startRestartGroup(<>) + val %dirty = %changed + if (%changed and 0b1110 == 0) { + %dirty = %dirty or if (%composer.changed(count)) 0b0100 else 0b0010 + } + if (%dirty and 0b1011 != 0b0010 || !%composer.skipping) { + Row(null, null, null, { %composer: Composer?, %changed: Int -> + %composer.startReplaceGroup(<>) + repeat(count) { it: Int -> + Text("A", %composer, 0b0110) + } + %composer.endReplaceGroup() + }, %composer, 0, 0b0111) + } else { + %composer.skipToGroupEnd() + } + %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int -> + Test(count, %composer, updateChangedFlags(%changed or 0b0001)) + } +} diff --git a/plugins/compose/compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt b/plugins/compose/compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt index 5c9d54478d517..3de368ac653bd 100644 --- a/plugins/compose/compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt +++ b/plugins/compose/compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt @@ -1000,6 +1000,12 @@ class ComposableFunctionBodyTransformer( val transformed = nonReturningBody.apply { transformChildrenVoid() }.let { + // Ensure that all group children of composable inline lambda are realized, since the inline + // lambda doesn't require a group on its own. + if (scope.isInlinedLambda && scope.isComposable) { + scope.realizeAllDirectChildren() + } + if (isInlineLambda) { it.asSourceOrEarlyExitGroup(scope) } else it @@ -2501,12 +2507,6 @@ class ComposableFunctionBodyTransformer( ) } - // Ensure that all group children of composable inline lambda are realized, since the inline - // lambda doesn't require a group on its own. - if (scope.isInlinedLambda && scope.isComposable) { - scope.realizeAllDirectChildren() - } - scope.realizeGroup(makeEnd) return when { // if the scope ends with a return call, then it will get properly ended if we