Make sure a composable call does not escape composable lambda

Compose compiler ignores inline lambdas when marking coalescable group children. This should not be the case for @composable inline lambdas, as they don't require additional groups to support the control flow. The coalescable group was previously added to both inline lambda block and parent composable block, incorrectly creating an additional endGroup call. Test: compose compiler tests Fixes: 331365999 ( https://issuetracker.google.com/issues/331365999 ) Change-Id: If5ca9550559263126252b923f7a4a9078b165d14 ( https://android-review.googlesource.com/q/If5ca9550559263126252b923f7a4a9078b165d14 ) Moved from: androidx/androidx@d4c18f0
JetBrains · Apr 19, 2024 · 154d479 · 154d479
1 parent aa362c8
commit 154d479
Show file tree

Hide file tree

Showing 6 changed files with 356 additions and 19 deletions.
diff --git a/.../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ControlFlowTransformTests.kt b/.../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ControlFlowTransformTests.kt
@@ -2477,4 +2477,64 @@ class ControlFlowTransformTests(useFir: Boolean) : AbstractControlFlowTransformT
  }
  """
  )
+
+ @Test
+ fun testConditionalReturnFromInline() = verifyGoldenComposeIrTransform(
+ extra = """
+ import androidx.compose.runtime.*
+
+ @Composable inline fun Column(content: @Composable () -> Unit) {}
+ inline fun NonComposable(content: () -> Unit) {}
+ @Composable fun Text(text: String) {}
+ """,
+ source = """
+ import androidx.compose.runtime.*
+
+ @Composable fun Test(test: Boolean) {
+ Column {
+ if (!test) {
+ Text("Say")
+ return@Column
+ }
+ Text("Hello")
+ }
+
+ NonComposable {
+ if (!test) {
+ Text("Say")
+ return@NonComposable
+ }
+ Text("Hello")
+ }
+ }
+ """
+ )
+
+ @Test
+ fun ifInsideInlineComposableFunction() = verifyGoldenComposeIrTransform(
+ extra = """
+ import androidx.compose.runtime.*
+
+ fun interface MeasurePolicy {
+ fun invoke(size: Int)
+ }
+ @Composable inline fun Layout(content: @Composable () -> Unit) {}
+ @Composable fun Box() {}
+ """,
+ source = """
+ import androidx.compose.runtime.*
+
+ @Composable
+ fun Label(test: Boolean) {
+ Layout(
+ content = {
+ Box()
+ if (test) {
+ Box()
+ }
+ }
+ )
+ }
+ """
+ )
 }
diff --git a/...ins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = false].txt b/...ins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = false].txt
@@ -0,0 +1,55 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.*
+
+@Composable
+fun Label(test: Boolean) {
+ Layout(
+ content = {
+ Box()
+ if (test) {
+ Box()
+ }
+ }
+ )
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Label(test: Boolean, %composer: Composer?, %changed: Int) {
+ %composer = %composer.startRestartGroup(<>)
+ sourceInformation(%composer, "C(Label)<Layout...>:Test.kt")
+ val %dirty = %changed
+ if (%changed and 0b0110 == 0) {
+ %dirty = %dirty or if (%composer.changed(test)) 0b0100 else 0b0010
+ }
+ if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+ if (isTraceInProgress()) {
+ traceEventStart(<>, %dirty, -1, <>)
+ }
+ Layout({ %composer: Composer?, %changed: Int ->
+ sourceInformationMarkerStart(%composer, <>, "C<Box()>:Test.kt")
+ Box(%composer, 0)
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "<Box()>")
+ if (test) {
+ Box(%composer, 0)
+ }
+ %composer.endReplaceGroup()
+ sourceInformationMarkerEnd(%composer)
+ }, %composer, 0)
+ if (isTraceInProgress()) {
+ traceEventEnd()
+ }
+ } else {
+ %composer.skipToGroupEnd()
+ }
+ %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+ Label(test, %composer, updateChangedFlags(%changed or 0b0001))
+ }
+}
diff --git a/...gins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = true].txt b/...gins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = true].txt
@@ -0,0 +1,55 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.*
+
+@Composable
+fun Label(test: Boolean) {
+ Layout(
+ content = {
+ Box()
+ if (test) {
+ Box()
+ }
+ }
+ )
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Label(test: Boolean, %composer: Composer?, %changed: Int) {
+ %composer = %composer.startRestartGroup(<>)
+ sourceInformation(%composer, "C(Label)<Layout...>:Test.kt")
+ val %dirty = %changed
+ if (%changed and 0b0110 == 0) {
+ %dirty = %dirty or if (%composer.changed(test)) 0b0100 else 0b0010
+ }
+ if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+ if (isTraceInProgress()) {
+ traceEventStart(<>, %dirty, -1, <>)
+ }
+ Layout({ %composer: Composer?, %changed: Int ->
+ sourceInformationMarkerStart(%composer, <>, "C<Box()>:Test.kt")
+ Box(%composer, 0)
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "<Box()>")
+ if (test) {
+ Box(%composer, 0)
+ }
+ %composer.endReplaceGroup()
+ sourceInformationMarkerEnd(%composer)
+ }, %composer, 0)
+ if (isTraceInProgress()) {
+ traceEventEnd()
+ }
+ } else {
+ %composer.skipToGroupEnd()
+ }
+ %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+ Label(test, %composer, updateChangedFlags(%changed or 0b0001))
+ }
+}
diff --git a/...gins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = false].txt b/...gins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = false].txt
@@ -0,0 +1,76 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.*
+
+@Composable fun Test(test: Boolean) {
+ Column {
+ if (!test) {
+ Text("Say")
+ return@Column
+ }
+ Text("Hello")
+ }
+
+ NonComposable {
+ if (!test) {
+ Text("Say")
+ return@NonComposable
+ }
+ Text("Hello")
+ }
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Test(test: Boolean, %composer: Composer?, %changed: Int) {
+ %composer = %composer.startRestartGroup(<>)
+ sourceInformation(%composer, "C(Test)<Column>,*<Text("...>:Test.kt")
+ val %dirty = %changed
+ if (%changed and 0b0110 == 0) {
+ %dirty = %dirty or if (%composer.changed(test)) 0b0100 else 0b0010
+ }
+ if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+ if (isTraceInProgress()) {
+ traceEventStart(<>, %dirty, -1, <>)
+ }
+ Column({ %composer: Composer?, %changed: Int ->
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "C<Text("...>:Test.kt")
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "<Text("...>")
+ if (!test) {
+ Text("Say", %composer, 0b0110)
+ %composer.endReplaceGroup()
+ %composer.endReplaceGroup()
+ return@Column
+ }
+ %composer.endReplaceGroup()
+ Text("Hello", %composer, 0b0110)
+ %composer.endReplaceGroup()
+ }, %composer, 0)
+ NonComposable {
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "<Text("...>")
+ if (!test) {
+ Text("Say", %composer, 0b0110)
+ %composer.endReplaceGroup()
+ return@NonComposable
+ }
+ %composer.endReplaceGroup()
+ Text("Hello", %composer, 0b0110)
+ }
+ if (isTraceInProgress()) {
+ traceEventEnd()
+ }
+ } else {
+ %composer.skipToGroupEnd()
+ }
+ %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+ Test(test, %composer, updateChangedFlags(%changed or 0b0001))
+ }
+}
diff --git a/...ugins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = true].txt b/...ugins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = true].txt
@@ -0,0 +1,76 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.*
+
+@Composable fun Test(test: Boolean) {
+ Column {
+ if (!test) {
+ Text("Say")
+ return@Column
+ }
+ Text("Hello")
+ }
+
+ NonComposable {
+ if (!test) {
+ Text("Say")
+ return@NonComposable
+ }
+ Text("Hello")
+ }
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Test(test: Boolean, %composer: Composer?, %changed: Int) {
+ %composer = %composer.startRestartGroup(<>)
+ sourceInformation(%composer, "C(Test)<Column>,*<Text("...>:Test.kt")
+ val %dirty = %changed
+ if (%changed and 0b0110 == 0) {
+ %dirty = %dirty or if (%composer.changed(test)) 0b0100 else 0b0010
+ }
+ if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+ if (isTraceInProgress()) {
+ traceEventStart(<>, %dirty, -1, <>)
+ }
+ Column({ %composer: Composer?, %changed: Int ->
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "C<Text("...>:Test.kt")
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "<Text("...>")
+ if (!test) {
+ Text("Say", %composer, 0b0110)
+ %composer.endReplaceGroup()
+ %composer.endReplaceGroup()
+ return@Column
+ }
+ %composer.endReplaceGroup()
+ Text("Hello", %composer, 0b0110)
+ %composer.endReplaceGroup()
+ }, %composer, 0)
+ NonComposable {
+ %composer.startReplaceGroup(<>)
+ sourceInformation(%composer, "<Text("...>")
+ if (!test) {
+ Text("Say", %composer, 0b0110)
+ %composer.endReplaceGroup()
+ return@NonComposable
+ }
+ %composer.endReplaceGroup()
+ Text("Hello", %composer, 0b0110)
+ }
+ if (isTraceInProgress()) {
+ traceEventEnd()
+ }
+ } else {
+ %composer.skipToGroupEnd()
+ }
+ %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+ Test(test, %composer, updateChangedFlags(%changed or 0b0001))
+ }
+}