[Compose] [nonSkippingGroupOptimization] Fixes group eliding for if s…

…tatements With nonSkippingGroupOptimization enabled the rules of when a group can be elided change. It is now required that groups generate a constant number of groups unless they are specifically looping groups which can only generate the body of the loop repeated, each iteration must be a single group. The groups for if statements were still being elided not following this rule. Fixes: [346821372](https://issuetracker.google.com/346821372) Relnote: Fixes group generation for if statements when nonSkippingGroupOptimziation is enabled.
JetBrains · Jun 28, 2024 · e207b05 · e207b05
1 parent 4307ba4
commit e207b05
Show file tree

Hide file tree

Showing 87 changed files with 882 additions and 355 deletions.
diff --git a/...ts/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/AbstractIrTransformTest.kt b/...ts/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/AbstractIrTransformTest.kt
@@ -33,9 +33,10 @@ abstract class AbstractIrTransformTest(useFir: Boolean) : AbstractCodegenTest(us
     override fun CompilerConfiguration.updateConfiguration() {
         put(ComposeConfiguration.SOURCE_INFORMATION_ENABLED_KEY, true)
         put(ComposeConfiguration.FEATURE_FLAGS, listOf(
-            FeatureFlag.StrongSkipping.featureName,
-            FeatureFlag.OptimizeNonSkippingGroups.featureName,
-        ))
+                FeatureFlag.StrongSkipping.featureName,
+                FeatureFlag.OptimizeNonSkippingGroups.featureName,
+            )
+        )
     }
 
     @JvmField

diff --git a/...est/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt b/...est/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt
@@ -1306,6 +1306,7 @@ class FunctionBodySkippingTransformTests(
                 fun ReceiveValue(value: Int) { }
             """
         )
+
 }
 
 class FunctionBodySkippingTransformTestsNoSource(
@@ -1314,6 +1315,13 @@ class FunctionBodySkippingTransformTestsNoSource(
     override fun CompilerConfiguration.updateConfiguration() {
         put(ComposeConfiguration.SOURCE_INFORMATION_ENABLED_KEY, false)
         put(ComposeConfiguration.TRACE_MARKERS_ENABLED_KEY, false)
+        put(
+            ComposeConfiguration.FEATURE_FLAGS,
+            listOf(
+                FeatureFlag.StrongSkipping.featureName,
+                FeatureFlag.OptimizeNonSkippingGroups.featureName,
+            )
+        )
     }
 
     @Test
@@ -1457,4 +1465,32 @@ class FunctionBodySkippingTransformTestsNoSource(
             fun Text(value: String) {}
         """
     )
+
+    @Test
+    fun testIfStatementGroups() = verifyGoldenComposeIrTransform(
+        source = """
+            import androidx.compose.runtime.*
+
+            @Composable
+            fun Test(level: Int) {
+                Wrap {
+                    if (level > 0) {
+                        used(remember { "Before" })
+                        Wrap {
+                            used(remember { "Middle" })
+                        }
+                        used(remember { "End" })
+                    }
+                }
+            }
+        """,
+        """
+            import androidx.compose.runtime.*
+
+            @Composable
+            fun Wrap(content: @Composable () -> Unit) = content()
+
+            fun used(value: Any) { }
+        """
+    )
 }
diff --git a/...ins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = false].txt b/...ins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = false].txt
@@ -35,12 +35,15 @@ fun Label(test: Boolean, %composer: Composer?, %changed: Int) {
     Layout({ %composer: Composer?, %changed: Int ->
       sourceInformationMarkerStart(%composer, <>, "C<Box()>:Test.kt")
       Box(%composer, 0)
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Box()>")
       if (test) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Box()>")
         Box(%composer, 0)
+        %composer.endReplaceGroup()
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
-      %composer.endReplaceGroup()
       sourceInformationMarkerEnd(%composer)
     }, %composer, 0)
     if (isTraceInProgress()) {

diff --git a/...gins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = true].txt b/...gins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = true].txt
@@ -35,12 +35,15 @@ fun Label(test: Boolean, %composer: Composer?, %changed: Int) {
     Layout({ %composer: Composer?, %changed: Int ->
       sourceInformationMarkerStart(%composer, <>, "C<Box()>:Test.kt")
       Box(%composer, 0)
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Box()>")
       if (test) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Box()>")
         Box(%composer, 0)
+        %composer.endReplaceGroup()
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
-      %composer.endReplaceGroup()
       sourceInformationMarkerEnd(%composer)
     }, %composer, 0)
     if (isTraceInProgress()) {

diff --git a/...idx.compose.compiler.plugins.kotlin.ControlFlowTransformTests/testAND[useFir = false].txt b/...idx.compose.compiler.plugins.kotlin.ControlFlowTransformTests/testAND[useFir = false].txt
@@ -26,8 +26,12 @@ fun Example(%composer: Composer?, %changed: Int) {
     traceEventStart(<>, %changed, -1, <>)
   }
   %composer.startReplaceGroup(<>)
-  sourceInformation(%composer, "<B()>,<B()>")
-  val tmp0_group = B(%composer, 0) && B(%composer, 0)
+  sourceInformation(%composer, "<B()>")
+  val tmp1_group = B(%composer, 0)
+  %composer.endReplaceGroup()
+  tmp1_group && %composer.startReplaceGroup(<>)
+  sourceInformation(%composer, "<B()>")
+  val tmp0_group = B(%composer, 0)
   %composer.endReplaceGroup()
   tmp0_group
   if (isTraceInProgress()) {

diff --git a/...oidx.compose.compiler.plugins.kotlin.ControlFlowTransformTests/testAND[useFir = true].txt b/...oidx.compose.compiler.plugins.kotlin.ControlFlowTransformTests/testAND[useFir = true].txt
@@ -26,8 +26,12 @@ fun Example(%composer: Composer?, %changed: Int) {
     traceEventStart(<>, %changed, -1, <>)
   }
   %composer.startReplaceGroup(<>)
-  sourceInformation(%composer, "<B()>,<B()>")
-  val tmp0_group = B(%composer, 0) && B(%composer, 0)
+  sourceInformation(%composer, "<B()>")
+  val tmp1_group = B(%composer, 0)
+  %composer.endReplaceGroup()
+  tmp1_group && %composer.startReplaceGroup(<>)
+  sourceInformation(%composer, "<B()>")
+  val tmp0_group = B(%composer, 0)
   %composer.endReplaceGroup()
   tmp0_group
   if (isTraceInProgress()) {

diff --git a/...gins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = false].txt b/...gins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = false].txt
@@ -41,27 +41,31 @@ fun Test(test: Boolean, %composer: Composer?, %changed: Int) {
     Column({ %composer: Composer?, %changed: Int ->
       %composer.startReplaceGroup(<>)
       sourceInformation(%composer, "C<Text("...>:Test.kt")
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Text("...>")
       if (!test) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Text("...>")
         Text("Say", %composer, 0b0110)
         %composer.endReplaceGroup()
         %composer.endReplaceGroup()
         return@Column
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
-      %composer.endReplaceGroup()
       Text("Hello", %composer, 0b0110)
       %composer.endReplaceGroup()
     }, %composer, 0)
     NonComposable {
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Text("...>")
       if (!test) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Text("...>")
         Text("Say", %composer, 0b0110)
         %composer.endReplaceGroup()
         return@NonComposable
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
-      %composer.endReplaceGroup()
       Text("Hello", %composer, 0b0110)
     }
     if (isTraceInProgress()) {

diff --git a/...ugins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = true].txt b/...ugins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = true].txt
@@ -41,27 +41,31 @@ fun Test(test: Boolean, %composer: Composer?, %changed: Int) {
     Column({ %composer: Composer?, %changed: Int ->
       %composer.startReplaceGroup(<>)
       sourceInformation(%composer, "C<Text("...>:Test.kt")
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Text("...>")
       if (!test) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Text("...>")
         Text("Say", %composer, 0b0110)
         %composer.endReplaceGroup()
         %composer.endReplaceGroup()
         return@Column
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
-      %composer.endReplaceGroup()
       Text("Hello", %composer, 0b0110)
       %composer.endReplaceGroup()
     }, %composer, 0)
     NonComposable {
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Text("...>")
       if (!test) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Text("...>")
         Text("Say", %composer, 0b0110)
         %composer.endReplaceGroup()
         return@NonComposable
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
-      %composer.endReplaceGroup()
       Text("Hello", %composer, 0b0110)
     }
     if (isTraceInProgress()) {

diff --git a/...lin.ControlFlowTransformTests/testDynamicWrappingGroupWithReturnValue[useFir = false].txt b/...lin.ControlFlowTransformTests/testDynamicWrappingGroupWithReturnValue[useFir = false].txt
@@ -28,25 +28,28 @@ fun Example(x: Int, %composer: Composer?, %changed: Int): Int {
   if (isTraceInProgress()) {
     traceEventStart(<>, %changed, -1, <>)
   }
-  val tmp0 = <block>{
+  val tmp0 = if (x > 0) {
     %composer.startReplaceGroup(<>)
     sourceInformation(%composer, "")
-    val tmp4_group = if (x > 0) {
-      val tmp3_group = if (%composer.startReplaceGroup(<>)
+    val tmp3_group = <block>{
+      if (%composer.startReplaceGroup(<>)
       sourceInformation(%composer, "<B()>")
       val tmp1_group = B(%composer, 0)
       %composer.endReplaceGroup()
       tmp1_group) 1 else if (%composer.startReplaceGroup(<>)
       sourceInformation(%composer, "<B()>")
       val tmp2_group = B(%composer, 0)
       %composer.endReplaceGroup()
-      tmp2_group) 2 else 3
-      tmp3_group
-    } else {
-      4
+      tmp2_group) 2 else if (%composer.startReplaceGroup(<>)
+      %composer.endReplaceGroup()
+      true) 3
     }
     %composer.endReplaceGroup()
-    tmp4_group
+    tmp3_group
+  } else {
+    %composer.startReplaceGroup(<>)
+    %composer.endReplaceGroup()
+    4
   }
   if (isTraceInProgress()) {
     traceEventEnd()

diff --git a/...tlin.ControlFlowTransformTests/testDynamicWrappingGroupWithReturnValue[useFir = true].txt b/...tlin.ControlFlowTransformTests/testDynamicWrappingGroupWithReturnValue[useFir = true].txt
@@ -28,25 +28,28 @@ fun Example(x: Int, %composer: Composer?, %changed: Int): Int {
   if (isTraceInProgress()) {
     traceEventStart(<>, %changed, -1, <>)
   }
-  val tmp0 = <block>{
+  val tmp0 = if (x > 0) {
     %composer.startReplaceGroup(<>)
     sourceInformation(%composer, "")
-    val tmp4_group = if (x > 0) {
-      val tmp3_group = if (%composer.startReplaceGroup(<>)
+    val tmp3_group = <block>{
+      if (%composer.startReplaceGroup(<>)
       sourceInformation(%composer, "<B()>")
       val tmp1_group = B(%composer, 0)
       %composer.endReplaceGroup()
       tmp1_group) 1 else if (%composer.startReplaceGroup(<>)
       sourceInformation(%composer, "<B()>")
       val tmp2_group = B(%composer, 0)
       %composer.endReplaceGroup()
-      tmp2_group) 2 else 3
-      tmp3_group
-    } else {
-      4
+      tmp2_group) 2 else if (%composer.startReplaceGroup(<>)
+      %composer.endReplaceGroup()
+      true) 3
     }
     %composer.endReplaceGroup()
-    tmp4_group
+    tmp3_group
+  } else {
+    %composer.startReplaceGroup(<>)
+    %composer.endReplaceGroup()
+    4
   }
   if (isTraceInProgress()) {
     traceEventEnd()

diff --git a/...ler.plugins.kotlin.ControlFlowTransformTests/testEarlyReturnCallValue[useFir = false].txt b/...ler.plugins.kotlin.ControlFlowTransformTests/testEarlyReturnCallValue[useFir = false].txt
@@ -28,18 +28,20 @@ fun Example(x: Int, %composer: Composer?, %changed: Int): Int {
   if (isTraceInProgress()) {
     traceEventStart(<>, %changed, -1, <>)
   }
-  %composer.startReplaceGroup(<>)
-  sourceInformation(%composer, "<R()>")
   if (x > 0) {
+    %composer.startReplaceGroup(<>)
+    sourceInformation(%composer, "<R()>")
     val tmp1_return = R(%composer, 0)
     %composer.endReplaceGroup()
     if (isTraceInProgress()) {
       traceEventEnd()
     }
     %composer.endReplaceGroup()
     return tmp1_return
+  } else {
+    %composer.startReplaceGroup(<>)
+    %composer.endReplaceGroup()
   }
-  %composer.endReplaceGroup()
   val tmp0 = R(%composer, 0)
   if (isTraceInProgress()) {
     traceEventEnd()

diff --git a/...iler.plugins.kotlin.ControlFlowTransformTests/testEarlyReturnCallValue[useFir = true].txt b/...iler.plugins.kotlin.ControlFlowTransformTests/testEarlyReturnCallValue[useFir = true].txt
@@ -28,18 +28,20 @@ fun Example(x: Int, %composer: Composer?, %changed: Int): Int {
   if (isTraceInProgress()) {
     traceEventStart(<>, %changed, -1, <>)
   }
-  %composer.startReplaceGroup(<>)
-  sourceInformation(%composer, "<R()>")
   if (x > 0) {
+    %composer.startReplaceGroup(<>)
+    sourceInformation(%composer, "<R()>")
     val tmp1_return = R(%composer, 0)
     %composer.endReplaceGroup()
     if (isTraceInProgress()) {
       traceEventEnd()
     }
     %composer.endReplaceGroup()
     return tmp1_return
+  } else {
+    %composer.startReplaceGroup(<>)
+    %composer.endReplaceGroup()
   }
-  %composer.endReplaceGroup()
   val tmp0 = R(%composer, 0)
   if (isTraceInProgress()) {
     traceEventEnd()

diff --git a/...otlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = false].txt b/...otlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = false].txt
@@ -30,13 +30,16 @@ private fun Test(param: String?, %composer: Composer?, %changed: Int) {
     Dialog({ %composer: Composer?, %changed: Int ->
       %composer.startReplaceGroup(<>)
       sourceInformation(%composer, "C:Test.kt")
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Test(p...>")
       if (false) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Test(p...>")
         Test(param, %composer, 0)
+        %composer.endReplaceGroup()
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
       %composer.endReplaceGroup()
-      %composer.endReplaceGroup()
     }, %composer, 0)
     if (isTraceInProgress()) {
       traceEventEnd()

diff --git a/...kotlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = true].txt b/...kotlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = true].txt
@@ -30,13 +30,16 @@ private fun Test(param: String?, %composer: Composer?, %changed: Int) {
     Dialog({ %composer: Composer?, %changed: Int ->
       %composer.startReplaceGroup(<>)
       sourceInformation(%composer, "C:Test.kt")
-      %composer.startReplaceGroup(<>)
-      sourceInformation(%composer, "<Test(p...>")
       if (false) {
+        %composer.startReplaceGroup(<>)
+        sourceInformation(%composer, "<Test(p...>")
         Test(param, %composer, 0)
+        %composer.endReplaceGroup()
+      } else {
+        %composer.startReplaceGroup(<>)
+        %composer.endReplaceGroup()
       }
       %composer.endReplaceGroup()
-      %composer.endReplaceGroup()
     }, %composer, 0)
     if (isTraceInProgress()) {
       traceEventEnd()