Prevent capturing param meta across crossinline boundary

Crossinline function can technically be in a different composition. Fixes: [343801379](https://issuetracker.google.com/343801379) Relnote: Stop capturing parameter meta across crossinline boundary.
JetBrains · Jun 10, 2024 · f38d5a3 · f38d5a3
1 parent 6253a64
commit f38d5a3
Show file tree

Hide file tree

Showing 8 changed files with 258 additions and 3 deletions.
diff --git a/...mTest/kotlin/androidx/compose/compiler/plugins/kotlin/StrongSkippingModeTransformTests.kt b/...mTest/kotlin/androidx/compose/compiler/plugins/kotlin/StrongSkippingModeTransformTests.kt
@@ -329,6 +329,45 @@ class StrongSkippingModeTransformTests(
         """
     )
 
+    @Test
+    fun unstableCrossinline() = verifyMemoization(
+        """
+            
+            // note: using var makes this class unstable (and this vm needs to be unstable for the crash to occur)
+            class MyUnstableViewModel(var text: String?) {
+                fun onClickyClicky() {}
+            }
+            
+            @Composable
+            fun Dialog(content: (@Composable () -> Unit)?) { content?.invoke() }
+
+            @Composable
+            fun Button(
+                onClick: () -> Unit
+            ) {}
+
+            inline fun <ValueT : Any> slotIfNotNull(
+                value: ValueT?,
+                crossinline slot: @Composable (ValueT) -> Unit
+            ): (@Composable () -> Unit)? {
+                return if (value != null) {
+                    @Composable { slot(value) }
+                } else null
+            }
+        """,
+        """
+            @Composable fun Scratch(vm: MyUnstableViewModel) {
+                Dialog(
+                    content = slotIfNotNull(vm.text) {
+                        Button(
+                            onClick = vm::onClickyClicky
+                        )
+                    }
+                )
+            }
+        """
+    )
+
     private fun verifyMemoization(
         @Language("kotlin")
         unchecked: String,

diff --git a/...otlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = false].txt b/...otlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = false].txt
@@ -33,7 +33,7 @@ private fun Test(param: String?, %composer: Composer?, %changed: Int) {
       %composer.startReplaceGroup(<>)
       sourceInformation(%composer, "<Test(p...>")
       if (false) {
-        Test(param, %composer, 0b1110 and %dirty)
+        Test(param, %composer, 0)
       }
       %composer.endReplaceGroup()
       %composer.endReplaceGroup()

diff --git a/...kotlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = true].txt b/...kotlin.ControlFlowTransformTests/testEarlyReturnFromCrossInlinedLambda[useFir = true].txt
@@ -33,7 +33,7 @@ private fun Test(param: String?, %composer: Composer?, %changed: Int) {
       %composer.startReplaceGroup(<>)
       sourceInformation(%composer, "<Test(p...>")
       if (false) {
-        Test(param, %composer, 0b1110 and %dirty)
+        Test(param, %composer, 0)
       }
       %composer.endReplaceGroup()
       %composer.endReplaceGroup()

diff --git a/...pingModeTransformTests/unstableCrossinline[useFir = false, intrinsicRemember = false].txt b/...pingModeTransformTests/unstableCrossinline[useFir = false, intrinsicRemember = false].txt
@@ -0,0 +1,54 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.NonRestartableComposable
+import androidx.compose.runtime.ReadOnlyComposable
+
+
+@Composable fun Scratch(vm: MyUnstableViewModel) {
+    Dialog(
+        content = slotIfNotNull(vm.text) {
+            Button(
+                onClick = vm::onClickyClicky
+            )
+        }
+    )
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Scratch(vm: MyUnstableViewModel, %composer: Composer?, %changed: Int) {
+  %composer = %composer.startRestartGroup(<>)
+  val %dirty = %changed
+  if (%changed and 0b0110 == 0) {
+    %dirty = %dirty or if (%composer.changedInstance(vm)) 0b0100 else 0b0010
+  }
+  if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+    if (isTraceInProgress()) {
+      traceEventStart(<>, %dirty, -1, <>)
+    }
+    Dialog(slotIfNotNull(vm.text) { it: String, %composer: Composer?, %changed: Int ->
+      %composer.startReplaceGroup(<>)
+      Button(<block>{
+        val tmp0 = vm
+        %composer.cache(%composer.changedInstance(tmp0)) {
+          tmp0::onClickyClicky
+        }
+      }, %composer, 0)
+      %composer.endReplaceGroup()
+    }, %composer, 0)
+    if (isTraceInProgress()) {
+      traceEventEnd()
+    }
+  } else {
+    %composer.skipToGroupEnd()
+  }
+  %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+    Scratch(vm, %composer, updateChangedFlags(%changed or 0b0001))
+  }
+}
diff --git a/...ppingModeTransformTests/unstableCrossinline[useFir = false, intrinsicRemember = true].txt b/...ppingModeTransformTests/unstableCrossinline[useFir = false, intrinsicRemember = true].txt
@@ -0,0 +1,54 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.NonRestartableComposable
+import androidx.compose.runtime.ReadOnlyComposable
+
+
+@Composable fun Scratch(vm: MyUnstableViewModel) {
+    Dialog(
+        content = slotIfNotNull(vm.text) {
+            Button(
+                onClick = vm::onClickyClicky
+            )
+        }
+    )
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Scratch(vm: MyUnstableViewModel, %composer: Composer?, %changed: Int) {
+  %composer = %composer.startRestartGroup(<>)
+  val %dirty = %changed
+  if (%changed and 0b0110 == 0) {
+    %dirty = %dirty or if (%composer.changedInstance(vm)) 0b0100 else 0b0010
+  }
+  if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+    if (isTraceInProgress()) {
+      traceEventStart(<>, %dirty, -1, <>)
+    }
+    Dialog(slotIfNotNull(vm.text) { it: String, %composer: Composer?, %changed: Int ->
+      %composer.startReplaceGroup(<>)
+      Button(<block>{
+        val tmp0 = vm
+        %composer.cache(%composer.changedInstance(tmp0)) {
+          tmp0::onClickyClicky
+        }
+      }, %composer, 0)
+      %composer.endReplaceGroup()
+    }, %composer, 0)
+    if (isTraceInProgress()) {
+      traceEventEnd()
+    }
+  } else {
+    %composer.skipToGroupEnd()
+  }
+  %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+    Scratch(vm, %composer, updateChangedFlags(%changed or 0b0001))
+  }
+}
diff --git a/...ppingModeTransformTests/unstableCrossinline[useFir = true, intrinsicRemember = false].txt b/...ppingModeTransformTests/unstableCrossinline[useFir = true, intrinsicRemember = false].txt
@@ -0,0 +1,54 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.NonRestartableComposable
+import androidx.compose.runtime.ReadOnlyComposable
+
+
+@Composable fun Scratch(vm: MyUnstableViewModel) {
+    Dialog(
+        content = slotIfNotNull(vm.text) {
+            Button(
+                onClick = vm::onClickyClicky
+            )
+        }
+    )
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Scratch(vm: MyUnstableViewModel, %composer: Composer?, %changed: Int) {
+  %composer = %composer.startRestartGroup(<>)
+  val %dirty = %changed
+  if (%changed and 0b0110 == 0) {
+    %dirty = %dirty or if (%composer.changedInstance(vm)) 0b0100 else 0b0010
+  }
+  if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+    if (isTraceInProgress()) {
+      traceEventStart(<>, %dirty, -1, <>)
+    }
+    Dialog(slotIfNotNull(vm.text) { it: String, %composer: Composer?, %changed: Int ->
+      %composer.startReplaceGroup(<>)
+      Button(<block>{
+        val tmp0 = vm
+        %composer.cache(%composer.changedInstance(tmp0)) {
+          tmp0::onClickyClicky
+        }
+      }, %composer, 0)
+      %composer.endReplaceGroup()
+    }, %composer, 0)
+    if (isTraceInProgress()) {
+      traceEventEnd()
+    }
+  } else {
+    %composer.skipToGroupEnd()
+  }
+  %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+    Scratch(vm, %composer, updateChangedFlags(%changed or 0b0001))
+  }
+}
diff --git a/...ippingModeTransformTests/unstableCrossinline[useFir = true, intrinsicRemember = true].txt b/...ippingModeTransformTests/unstableCrossinline[useFir = true, intrinsicRemember = true].txt
@@ -0,0 +1,54 @@
+//
+// Source
+// ------------------------------------------
+
+import androidx.compose.runtime.Composable
+import androidx.compose.runtime.NonRestartableComposable
+import androidx.compose.runtime.ReadOnlyComposable
+
+
+@Composable fun Scratch(vm: MyUnstableViewModel) {
+    Dialog(
+        content = slotIfNotNull(vm.text) {
+            Button(
+                onClick = vm::onClickyClicky
+            )
+        }
+    )
+}
+
+//
+// Transformed IR
+// ------------------------------------------
+
+@Composable
+fun Scratch(vm: MyUnstableViewModel, %composer: Composer?, %changed: Int) {
+  %composer = %composer.startRestartGroup(<>)
+  val %dirty = %changed
+  if (%changed and 0b0110 == 0) {
+    %dirty = %dirty or if (%composer.changedInstance(vm)) 0b0100 else 0b0010
+  }
+  if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
+    if (isTraceInProgress()) {
+      traceEventStart(<>, %dirty, -1, <>)
+    }
+    Dialog(slotIfNotNull(vm.text) { it: String, %composer: Composer?, %changed: Int ->
+      %composer.startReplaceGroup(<>)
+      Button(<block>{
+        val tmp0 = vm
+        %composer.cache(%composer.changedInstance(tmp0)) {
+          tmp0::onClickyClicky
+        }
+      }, %composer, 0)
+      %composer.endReplaceGroup()
+    }, %composer, 0)
+    if (isTraceInProgress()) {
+      traceEventEnd()
+    }
+  } else {
+    %composer.skipToGroupEnd()
+  }
+  %composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
+    Scratch(vm, %composer, updateChangedFlags(%changed or 0b0001))
+  }
+}
diff --git a/.../java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt b/.../java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt
@@ -2827,7 +2827,7 @@ class ComposableFunctionBodyTransformer(
                         return null
                     } else {
                         // If the capture is outside inline lambda, we don't allow meta propagation
-                        if (!inlineLambdaInfo.isInlineLambda(scope.function)) {
+                        if (!inlineLambdaInfo.isInlineLambda(scope.function) || inlineLambdaInfo.isCrossinlineLambda(scope.function)) {
                             return null
                         }
                     }