Add source markers to @ExplicitGroupsComposable

ShikaSD · Space Cloud · commit 4bad11daee43 · 2024-09-25T09:46:43.000Z
Also removes inner groups around conditions from explicit group composable functions.
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
@@ -1307,6 +1307,29 @@ class FunctionBodySkippingTransformTests(
             """
         )
 
+    @Test
+    fun testExplicitGroups(): Unit = comparisonPropagation(
+        """
+            class Foo
+        """,
+        """
+            import androidx.compose.runtime.*
+
+            @Composable
+            @ExplicitGroupsComposable
+            inline fun ReusableContentHost(active: Boolean, crossinline content: @Composable () -> Unit) {
+                currentComposer.startReusableGroup(200, active)
+                val activeChanged = currentComposer.changed(active)
+                if (active) {
+                    content()
+                } else {
+                    currentComposer.deactivateToEndGroup(activeChanged)
+                }
+                currentComposer.endReusableGroup()
+            }
+        """
+    )
+
 }
 
 class FunctionBodySkippingTransformTestsNoSource(
diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testExplicitGroups[useFir = false].txt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testExplicitGroups[useFir = false].txt
@@ -0,0 +1,43 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.NonRestartableComposable
+import androidx.compose.runtime.ReadOnlyComposable
+
+
+import androidx.compose.runtime.*
+
+@Composable
+@ExplicitGroupsComposable
+inline fun ReusableContentHost(active: Boolean, crossinline content: @Composable () -> Unit) {
+    currentComposer.startReusableGroup(200, active)
+    val activeChanged = currentComposer.changed(active)
+    if (active) {
+        content()
+    } else {
+        currentComposer.deactivateToEndGroup(activeChanged)
+    }
+    currentComposer.endReusableGroup()
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+@ExplicitGroupsComposable
+@ComposableInferredTarget(scheme = "[0[0]]")
+fun ReusableContentHost(active: Boolean, crossinline content: Function2<Composer, Int, Unit>, %composer: Composer?, %changed: Int) {
+  sourceInformationMarkerStart(%composer, <>, "CC(ReusableContentHost)<conten...>:Test.kt")
+  %composer.startReusableGroup(200, active)
+  val activeChanged = %composer.changed(active)
+  if (active) {
+    content(%composer, 0b1110 and %changed shr 0b0011)
+  } else {
+    %composer.deactivateToEndGroup(activeChanged)
+  }
+  %composer.endReusableGroup()
+  sourceInformationMarkerEnd(%composer)
+}
diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testExplicitGroups[useFir = true].txt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testExplicitGroups[useFir = true].txt
@@ -0,0 +1,43 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.NonRestartableComposable
+import androidx.compose.runtime.ReadOnlyComposable
+
+
+import androidx.compose.runtime.*
+
+@Composable
+@ExplicitGroupsComposable
+inline fun ReusableContentHost(active: Boolean, crossinline content: @Composable () -> Unit) {
+    currentComposer.startReusableGroup(200, active)
+    val activeChanged = currentComposer.changed(active)
+    if (active) {
+        content()
+    } else {
+        currentComposer.deactivateToEndGroup(activeChanged)
+    }
+    currentComposer.endReusableGroup()
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+@ExplicitGroupsComposable
+@ComposableInferredTarget(scheme = "[0[0]]")
+fun ReusableContentHost(active: Boolean, crossinline content: Function2<Composer, Int, Unit>, %composer: Composer?, %changed: Int) {
+  sourceInformationMarkerStart(%composer, <>, "CC(ReusableContentHost)<conten...>:Test.kt")
+  %composer.startReusableGroup(200, active)
+  val activeChanged = %composer.changed(active)
+  if (active) {
+    content(%composer, 0b1110 and %changed shr 0b0011)
+  } else {
+    %composer.deactivateToEndGroup(activeChanged)
+  }
+  %composer.endReusableGroup()
+  sourceInformationMarkerEnd(%composer)
+}
diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testReceiverIssue[useFir = false].txt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testReceiverIssue[useFir = false].txt
@@ -34,24 +34,30 @@ inline fun c(foo: Foo, label: String) {
 @Composable
 @ExplicitGroupsComposable
 fun A(foo: Foo, %composer: Composer?, %changed: Int) {
+  sourceInformationMarkerStart(%composer, <>, "C(A)<b()>:Test.kt")
   if (isTraceInProgress()) {
     traceEventStart(<>, %changed, -1, <>)
   }
   foo.b(null, %composer, 0b1110 and %changed, 0b0001)
   if (isTraceInProgress()) {
     traceEventEnd()
   }
+  sourceInformationMarkerEnd(%composer)
 }
 @Composable
 @ExplicitGroupsComposable
 fun Foo.b(label: String?, %composer: Composer?, %changed: Int, %default: Int) {
+  sourceInformationMarkerStart(%composer, <>, "CC(b)<c(this...>:Test.kt")
   if (%default and 0b0001 != 0) {
     label = ""
   }
   c(<this>, label, %composer, 0b1110 and %changed or 0b01110000 and %changed)
+  sourceInformationMarkerEnd(%composer)
 }
 @Composable
 @ExplicitGroupsComposable
 fun c(foo: Foo, label: String, %composer: Composer?, %changed: Int) {
+  sourceInformationMarkerStart(%composer, <>, "CC(c):Test.kt")
   print(label)
+  sourceInformationMarkerEnd(%composer)
 }
diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testReceiverIssue[useFir = true].txt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTests/testReceiverIssue[useFir = true].txt
@@ -34,24 +34,30 @@ inline fun c(foo: Foo, label: String) {
 @Composable
 @ExplicitGroupsComposable
 fun A(foo: Foo, %composer: Composer?, %changed: Int) {
+  sourceInformationMarkerStart(%composer, <>, "C(A)<b()>:Test.kt")
   if (isTraceInProgress()) {
     traceEventStart(<>, %changed, -1, <>)
   }
   foo.b(null, %composer, 0b1110 and %changed, 0b0001)
   if (isTraceInProgress()) {
     traceEventEnd()
   }
+  sourceInformationMarkerEnd(%composer)
 }
 @Composable
 @ExplicitGroupsComposable
 fun Foo.b(label: String?, %composer: Composer?, %changed: Int, %default: Int) {
+  sourceInformationMarkerStart(%composer, <>, "CC(b)<c(this...>:Test.kt")
   if (%default and 0b0001 != 0) {
     label = ""
   }
   c(<this>, label, %composer, 0b1110 and %changed or 0b01110000 and %changed)
+  sourceInformationMarkerEnd(%composer)
 }
 @Composable
 @ExplicitGroupsComposable
 fun c(foo: Foo, label: String, %composer: Composer?, %changed: Int) {
+  sourceInformationMarkerStart(%composer, <>, "CC(c):Test.kt")
   print(label)
+  sourceInformationMarkerEnd(%composer)
 }
diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/ensureNoGroupsAreAddedToAnExplicitGroupsComposable[useFir = false].txt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/ensureNoGroupsAreAddedToAnExplicitGroupsComposable[useFir = false].txt
@@ -25,12 +25,10 @@ inline fun Test(active: Boolean, content: @Composable () -> Unit) {
 @ComposableInferredTarget(scheme = "[0[0]]")
 fun Test(active: Boolean, content: Function2<Composer, Int, Unit>, %composer: Composer?, %changed: Int) {
   %composer.startReusableGroup(1, null)
-  %composer.startReplaceGroup(<>)
   if (active) {
     content(%composer, 0b1110 and %changed shr 0b0011)
   } else {
     %composer.deactivateToEndGroup(false)
   }
-  %composer.endReplaceGroup()
   %composer.endReusableGroup()
 }
diff --git a/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/ensureNoGroupsAreAddedToAnExplicitGroupsComposable[useFir = true].txt b/plugins/compose/compiler-hosted/integration-tests/src/jvmTest/resources/golden/androidx.compose.compiler.plugins.kotlin.FunctionBodySkippingTransformTestsNoSource/ensureNoGroupsAreAddedToAnExplicitGroupsComposable[useFir = true].txt
@@ -25,12 +25,10 @@ inline fun Test(active: Boolean, content: @Composable () -> Unit) {
 @ComposableInferredTarget(scheme = "[0[0]]")
 fun Test(active: Boolean, content: Function2<Composer, Int, Unit>, %composer: Composer?, %changed: Int) {
   %composer.startReusableGroup(1, null)
-  %composer.startReplaceGroup(<>)
   if (active) {
     content(%composer, 0b1110 and %changed shr 0b0011)
   } else {
     %composer.deactivateToEndGroup(false)
   }
-  %composer.endReplaceGroup()
   %composer.endReusableGroup()
 }
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
@@ -750,8 +750,7 @@ class ComposableFunctionBodyTransformer(
         val body = declaration.body!!
 
         val hasExplicitGroups = declaration.hasExplicitGroups
-        val isReadOnly = declaration.hasReadOnlyAnnotation ||
-            declaration.isComposableDelegatedAccessor()
+        val isReadOnly = declaration.hasReadOnlyAnnotation || declaration.isComposableDelegatedAccessor()
 
         // An outer group is required if we are a lambda or dynamic method or the runtime doesn't
         // support remember after call. A outer group is explicitly elided by readonly and has
@@ -822,7 +821,7 @@ class ComposableFunctionBodyTransformer(
                                 scope,
                                 irFunctionSourceKey()
                             )
-                        collectSourceInformation && !hasExplicitGroups ->
+                        collectSourceInformation ->
                             irSourceInformationMarkerStart(
                                 body,
                                 scope,
@@ -835,7 +834,7 @@ class ComposableFunctionBodyTransformer(
                     *transformed.statements.toTypedArray(),
                     when {
                         outerGroupRequired -> irEndReplaceGroup(scope = scope)
-                        collectSourceInformation && !hasExplicitGroups ->
+                        collectSourceInformation ->
                             irSourceInformationMarkerEnd(body, scope)
                         else -> null
                     },
@@ -844,7 +843,7 @@ class ComposableFunctionBodyTransformer(
             )
         }
 
-        if (!outerGroupRequired && !hasExplicitGroups) {
+        if (!outerGroupRequired) {
             scope.realizeEndCalls {
                 irComposite(
                     statements = listOfNotNull(
@@ -3703,8 +3702,9 @@ class ComposableFunctionBodyTransformer(
 
     override fun visitWhen(expression: IrWhen): IrExpression {
         if (!isInComposableScope) return super.visitWhen(expression)
+        if (currentFunctionScope.function.hasExplicitGroups) return super.visitWhen(expression)
 
-        val optimizeGroups = FeatureFlag.OptimizeNonSkippingGroups.enabled && !currentFunctionScope.function.hasExplicitGroups
+        val optimizeGroups = FeatureFlag.OptimizeNonSkippingGroups.enabled
 
         // Composable calls in conditions are more expensive than composable calls in the different
         // result branches of the when clause. This is because if we have N branches of a when

Original file line number	Diff line number	Diff line change
`@@ -34,24 +34,30 @@ inline fun c(foo: Foo, label: String) {`
`34`	`34`	`@Composable`
`35`	`35`	`@ExplicitGroupsComposable`
`36`	`36`	`fun A(foo: Foo, %composer: Composer?, %changed: Int) {`
	`37`	`+ sourceInformationMarkerStart(%composer, <>, "C(A)<b()>:Test.kt")`
`37`	`38`	`if (isTraceInProgress()) {`
`38`	`39`	`traceEventStart(<>, %changed, -1, <>)`
`39`	`40`	`}`
`40`	`41`	`foo.b(null, %composer, 0b1110 and %changed, 0b0001)`
`41`	`42`	`if (isTraceInProgress()) {`
`42`	`43`	`traceEventEnd()`
`43`	`44`	`}`
	`45`	`+ sourceInformationMarkerEnd(%composer)`
`44`	`46`	`}`
`45`	`47`	`@Composable`
`46`	`48`	`@ExplicitGroupsComposable`
`47`	`49`	`fun Foo.b(label: String?, %composer: Composer?, %changed: Int, %default: Int) {`
	`50`	`+ sourceInformationMarkerStart(%composer, <>, "CC(b)<c(this...>:Test.kt")`
`48`	`51`	`if (%default and 0b0001 != 0) {`
`49`	`52`	`label = ""`
`50`	`53`	`}`
`51`	`54`	`c(<this>, label, %composer, 0b1110 and %changed or 0b01110000 and %changed)`
	`55`	`+ sourceInformationMarkerEnd(%composer)`
`52`	`56`	`}`
`53`	`57`	`@Composable`
`54`	`58`	`@ExplicitGroupsComposable`
`55`	`59`	`fun c(foo: Foo, label: String, %composer: Composer?, %changed: Int) {`
	`60`	`+ sourceInformationMarkerStart(%composer, <>, "CC(c):Test.kt")`
`56`	`61`	`print(label)`
	`62`	`+ sourceInformationMarkerEnd(%composer)`
`57`	`63`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,12 +25,10 @@ inline fun Test(active: Boolean, content: @Composable () -> Unit) {`
`25`	`25`	`@ComposableInferredTarget(scheme = "[0[0]]")`
`26`	`26`	`fun Test(active: Boolean, content: Function2<Composer, Int, Unit>, %composer: Composer?, %changed: Int) {`
`27`	`27`	`%composer.startReusableGroup(1, null)`
`28`		`- %composer.startReplaceGroup(<>)`
`29`	`28`	`if (active) {`
`30`	`29`	`content(%composer, 0b1110 and %changed shr 0b0011)`
`31`	`30`	`} else {`
`32`	`31`	`%composer.deactivateToEndGroup(false)`
`33`	`32`	`}`
`34`		`- %composer.endReplaceGroup()`
`35`	`33`	`%composer.endReusableGroup()`
`36`	`34`	`}`