Add source markers to @ExplicitGroupsComposable

Also removes inner groups around conditions from explicit group composable functions.
JetBrains · Sep 25, 2024 · 4bad11d · 4bad11d
1 parent fcc1075
commit 4bad11d
Show file tree

Hide file tree

Showing 8 changed files with 127 additions and 10 deletions.
diff --git a/...est/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt b/...est/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.kotlin.FunctionBodySkippingTransformTests/testExplicitGroups[useFir = false].txt b/....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/...r.plugins.kotlin.FunctionBodySkippingTransformTests/testExplicitGroups[useFir = true].txt b/...r.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/...r.plugins.kotlin.FunctionBodySkippingTransformTests/testReceiverIssue[useFir = false].txt b/...r.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/...er.plugins.kotlin.FunctionBodySkippingTransformTests/testReceiverIssue[useFir = true].txt b/...er.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/...sformTestsNoSource/ensureNoGroupsAreAddedToAnExplicitGroupsComposable[useFir = false].txt b/...sformTestsNoSource/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/...nsformTestsNoSource/ensureNoGroupsAreAddedToAnExplicitGroupsComposable[useFir = true].txt b/...nsformTestsNoSource/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/.../java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt b/.../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