Skip to content

Commit 8f0eeb1

Browse files
ShikaSDSpace Cloud
authored and
Space Cloud
committed
Realize groups in when expr if a wrapping group is omitted
Test: compiler tests Fixes: 336571300 ( https://issuetracker.google.com/issues/336571300 ) Change-Id: I1ce2de38d9b13528a2039528d69e7c10854329fa ( https://android-review.googlesource.com/q/I1ce2de38d9b13528a2039528d69e7c10854329fa ) Moved from: androidx/androidx@9ab1db3
1 parent 868d0ac commit 8f0eeb1

File tree

6 files changed

+165
-5
lines changed

6 files changed

+165
-5
lines changed

plugins/compose/compiler-hosted/integration-tests/src/androidUnitTest/kotlin/androidx/compose/compiler/plugins/kotlin/ComposeBytecodeCodegenTest.kt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,4 +811,24 @@ class ComposeBytecodeCodegenTest(useFir: Boolean) : AbstractCodegenTest(useFir)
811811
}
812812
},
813813
)
814+
815+
@Test // regression test for 336571300
816+
fun test_groupAroundIfComposeCallInIfConditionWithShortCircuit() = testCompile(
817+
source = """
818+
import androidx.compose.runtime.*
819+
820+
@Composable
821+
fun Test() {
822+
ReceiveValue(if (state && getCondition()) 0 else 1)
823+
}
824+
825+
val state by mutableStateOf(true)
826+
827+
@Composable
828+
fun getCondition() = remember { mutableStateOf(false) }.value
829+
830+
@Composable
831+
fun ReceiveValue(value: Int) { }
832+
"""
833+
)
814834
}

plugins/compose/compiler-hosted/integration-tests/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/FunctionBodySkippingTransformTests.kt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1281,6 +1281,30 @@ class FunctionBodySkippingTransformTests(
12811281
fun Text(value: String) {}
12821282
"""
12831283
)
1284+
1285+
@Test // regression test for 336571300
1286+
fun test_groupAroundIfComposeCallInIfConditionWithShortCircuit() =
1287+
verifyGoldenComposeIrTransform(
1288+
source = """
1289+
import androidx.compose.runtime.*
1290+
1291+
@Composable
1292+
fun Test() {
1293+
ReceiveValue(if (state && getCondition()) 0 else 1)
1294+
}
1295+
""",
1296+
"""
1297+
import androidx.compose.runtime.*
1298+
1299+
val state by mutableStateOf(true)
1300+
1301+
@Composable
1302+
fun getCondition() = remember { mutableStateOf(false) }.value
1303+
1304+
@Composable
1305+
fun ReceiveValue(value: Int) { }
1306+
"""
1307+
)
12841308
}
12851309

12861310
class FunctionBodySkippingTransformTestsNoSource(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//
2+
// Source
3+
// ------------------------------------------
4+
5+
import androidx.compose.runtime.*
6+
7+
@Composable
8+
fun Test() {
9+
ReceiveValue(if (state && getCondition()) 0 else 1)
10+
}
11+
12+
//
13+
// Transformed IR
14+
// ------------------------------------------
15+
16+
@Composable
17+
fun Test(%composer: Composer?, %changed: Int) {
18+
%composer = %composer.startRestartGroup(<>)
19+
sourceInformation(%composer, "C(Test)<Receiv...>:Test.kt")
20+
if (%changed != 0 || !%composer.skipping) {
21+
if (isTraceInProgress()) {
22+
traceEventStart(<>, %changed, -1, <>)
23+
}
24+
ReceiveValue(if (%composer.startReplaceGroup(<>)
25+
sourceInformation(%composer, "<getCon...>")
26+
val tmp0_group = state && getCondition(%composer, 0)
27+
%composer.endReplaceGroup()
28+
tmp0_group) 0 else 1, %composer, 0)
29+
if (isTraceInProgress()) {
30+
traceEventEnd()
31+
}
32+
} else {
33+
%composer.skipToGroupEnd()
34+
}
35+
%composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
36+
Test(%composer, updateChangedFlags(%changed or 0b0001))
37+
}
38+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//
2+
// Source
3+
// ------------------------------------------
4+
5+
import androidx.compose.runtime.*
6+
7+
@Composable
8+
fun Test() {
9+
ReceiveValue(if (state && getCondition()) 0 else 1)
10+
}
11+
12+
//
13+
// Transformed IR
14+
// ------------------------------------------
15+
16+
@Composable
17+
fun Test(%composer: Composer?, %changed: Int) {
18+
%composer = %composer.startRestartGroup(<>)
19+
sourceInformation(%composer, "C(Test)<Receiv...>:Test.kt")
20+
if (%changed != 0 || !%composer.skipping) {
21+
if (isTraceInProgress()) {
22+
traceEventStart(<>, %changed, -1, <>)
23+
}
24+
ReceiveValue(if (%composer.startReplaceGroup(<>)
25+
sourceInformation(%composer, "<getCon...>")
26+
val tmp0_group = state && getCondition(%composer, 0)
27+
%composer.endReplaceGroup()
28+
tmp0_group) 0 else 1, %composer, 0)
29+
if (isTraceInProgress()) {
30+
traceEventEnd()
31+
}
32+
} else {
33+
%composer.skipToGroupEnd()
34+
}
35+
%composer.endRestartGroup()?.updateScope { %composer: Composer?, %force: Int ->
36+
Test(%composer, updateChangedFlags(%changed or 0b0001))
37+
}
38+
}

plugins/compose/compiler-hosted/runtime-tests/src/commonTest/kotlin/androidx/compose/compiler/test/CompositionTests.kt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package androidx.compose.compiler.test
1818

1919
import androidx.compose.runtime.Composable
20+
import androidx.compose.runtime.NonRestartableComposable
2021
import androidx.compose.runtime.getValue
2122
import androidx.compose.runtime.mock.Text
2223
import androidx.compose.runtime.mock.compositionTest
@@ -26,6 +27,7 @@ import androidx.compose.runtime.remember
2627
import androidx.compose.runtime.rememberUpdatedState
2728
import androidx.compose.runtime.setValue
2829
import kotlin.test.Test
30+
import kotlin.test.assertEquals
2931

3032
class CompositionTests {
3133
@Test
@@ -80,6 +82,37 @@ class CompositionTests {
8082
DefaultValueClass()
8183
}
8284
}
85+
86+
@Test
87+
fun groupAroundIfComposeCallInIfConditionWithShortCircuit() = compositionTest {
88+
var state by mutableStateOf(true)
89+
compose {
90+
ReceiveValue(if (state && getCondition()) 0 else 1)
91+
92+
ReceiveValue(
93+
when {
94+
state -> when {
95+
state -> getCondition()
96+
else -> false
97+
}.let { if (it) 0 else 1 }
98+
else -> 1
99+
}
100+
)
101+
}
102+
103+
state = false
104+
advance()
105+
}
106+
}
107+
108+
@Composable
109+
fun getCondition() = remember { false }
110+
111+
@NonRestartableComposable
112+
@Composable
113+
fun ReceiveValue(value: Int) {
114+
val string = remember { "$value" }
115+
assertEquals(1, string.length)
83116
}
84117

85118
class CrossInlineState(content: @Composable () -> Unit = { }) {

plugins/compose/compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3892,12 +3892,19 @@ class ComposableFunctionBodyTransformer(
38923892
}
38933893

38943894
forEachWith(transformed.branches, condScopes, resultScopes) { it, condScope, resultScope ->
3895-
// If the conditional block doesn't have a composable call in it, we don't need
3896-
// to generate a group around it because we will be generating one around the entire
3897-
// if statement
3898-
if (needsWrappingGroup && condScope.hasComposableCalls) {
3899-
it.condition = it.condition.asReplaceGroup(condScope)
3895+
if (condScope.hasComposableCalls) {
3896+
if (needsWrappingGroup) {
3897+
// Generate a group around the conditional block when it has a composable call
3898+
// in it and we are generating a group around when block.
3899+
it.condition = it.condition.asReplaceGroup(condScope)
3900+
} else {
3901+
// Ensure that the inner structure of condition is correct if the wrapping group
3902+
// is not required by realizing groups in condition scope.
3903+
condScope.realizeAllDirectChildren()
3904+
condScope.realizeCoalescableGroup()
3905+
}
39003906
}
3907+
39013908
if (
39023909
// if no wrapping group but more than one result have calls, we have to have every
39033910
// result be a group so that we have a consistent number of groups during execution

0 commit comments

Comments
 (0)