Skip to content

Commit

Permalink
Make sure a composable call does not escape composable lambda
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@ShikaSD
ShikaSD authored and Space Cloud committed Apr 19, 2024
1 parent aa362c8 commit 154d479
Show file tree
Hide file tree
Showing 6 changed files with 356 additions and 19 deletions.
60 changes: 60 additions & 0 deletions .../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ControlFlowTransformTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
)
}
"""
)
}
55 changes: 55 additions & 0 deletions ...ins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = false].txt
View file
Original file line number Diff line number Diff line change
@@ -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))
}
}
55 changes: 55 additions & 0 deletions ...gins.kotlin.ControlFlowTransformTests/ifInsideInlineComposableFunction[useFir = true].txt
View file
Original file line number Diff line number Diff line change
@@ -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))
}
}
76 changes: 76 additions & 0 deletions ...gins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = false].txt
View file
Original file line number Diff line number Diff line change
@@ -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))
}
}
76 changes: 76 additions & 0 deletions ...ugins.kotlin.ControlFlowTransformTests/testConditionalReturnFromInline[useFir = true].txt
View file
Original file line number Diff line number Diff line change
@@ -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))
}
}
Loading

0 comments on commit 154d479

Please sign in to comment.