Move source information of default parameters to the default group

JetBrains · Sep 25, 2024 · 9fcd247 · 9fcd247
1 parent 4bad11d
commit 9fcd247
Show file tree

Hide file tree

Showing 12 changed files with 79 additions and 63 deletions.
diff --git a/...ins.kotlin.FunctionBodySkippingTransformTests/testComposableParameter[useFir = false].txt b/...ins.kotlin.FunctionBodySkippingTransformTests/testComposableParameter[useFir = false].txt
@@ -21,7 +21,7 @@ fun Example(a: Int = 0, b: Int = makeInt(), c: Int = 0) {
 @Composable
 fun Example(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Example)<makeIn...>:Test.kt")
+  sourceInformation(%composer, "C(Example):Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -38,6 +38,7 @@ fun Example(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %defaul
   }
   if (%dirty and 0b10010011 != 0b10010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<makeIn...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         a = 0

diff --git a/...gins.kotlin.FunctionBodySkippingTransformTests/testComposableParameter[useFir = true].txt b/...gins.kotlin.FunctionBodySkippingTransformTests/testComposableParameter[useFir = true].txt
@@ -21,7 +21,7 @@ fun Example(a: Int = 0, b: Int = makeInt(), c: Int = 0) {
 @Composable
 fun Example(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Example)<makeIn...>:Test.kt")
+  sourceInformation(%composer, "C(Example):Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -38,6 +38,7 @@ fun Example(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %defaul
   }
   if (%dirty and 0b10010011 != 0b10010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<makeIn...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         a = 0

diff --git a/...BodySkippingTransformTests/testSingleStableParamWithComposableDefault[useFir = false].txt b/...BodySkippingTransformTests/testSingleStableParamWithComposableDefault[useFir = false].txt
@@ -19,13 +19,14 @@ fun Test(x: Int = I()) {
 @Composable
 fun Test(x: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Test)<I()>,<A(x)>:Test.kt")
+  sourceInformation(%composer, "C(Test)<A(x)>:Test.kt")
   val %dirty = %changed
   if (%changed and 0b0110 == 0) {
     %dirty = %dirty or if (%default and 0b0001 == 0 && %composer.changed(x)) 0b0100 else 0b0010
   }
   if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<I()>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         x = I(%composer, 0)

diff --git a/...nBodySkippingTransformTests/testSingleStableParamWithComposableDefault[useFir = true].txt b/...nBodySkippingTransformTests/testSingleStableParamWithComposableDefault[useFir = true].txt
@@ -19,13 +19,14 @@ fun Test(x: Int = I()) {
 @Composable
 fun Test(x: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Test)<I()>,<A(x)>:Test.kt")
+  sourceInformation(%composer, "C(Test)<A(x)>:Test.kt")
   val %dirty = %changed
   if (%changed and 0b0110 == 0) {
     %dirty = %dirty or if (%default and 0b0001 == 0 && %composer.changed(x)) 0b0100 else 0b0010
   }
   if (%dirty and 0b0011 != 0b0010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<I()>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         x = I(%composer, 0)

diff --git a/...BodySkippingTransformTests/testStaticAndNonStaticDefaultValueSkipping[useFir = false].txt b/...BodySkippingTransformTests/testStaticAndNonStaticDefaultValueSkipping[useFir = false].txt
@@ -23,7 +23,7 @@ fun Example(
 @Composable
 fun Example(wontChange: Int, mightChange: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Example)P(1)<curren...>,<A(wont...>,<A(migh...>:Test.kt")
+  sourceInformation(%composer, "C(Example)P(1)<A(wont...>,<A(migh...>:Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -35,6 +35,7 @@ fun Example(wontChange: Int, mightChange: Int, %composer: Composer?, %changed: I
   }
   if (%dirty and 0b00010011 != 0b00010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<curren...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         wontChange = 123

diff --git a/...nBodySkippingTransformTests/testStaticAndNonStaticDefaultValueSkipping[useFir = true].txt b/...nBodySkippingTransformTests/testStaticAndNonStaticDefaultValueSkipping[useFir = true].txt
@@ -23,7 +23,7 @@ fun Example(
 @Composable
 fun Example(wontChange: Int, mightChange: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Example)P(1)<curren...>,<A(wont...>,<A(migh...>:Test.kt")
+  sourceInformation(%composer, "C(Example)P(1)<A(wont...>,<A(migh...>:Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -35,6 +35,7 @@ fun Example(wontChange: Int, mightChange: Int, %composer: Composer?, %changed: I
   }
   if (%dirty and 0b00010011 != 0b00010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<curren...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         wontChange = 123

diff --git a/...ansformTests/testIntrinsicRememberOfDefaultParameters_AfterComposable[useFir = false].txt b/...ansformTests/testIntrinsicRememberOfDefaultParameters_AfterComposable[useFir = false].txt
@@ -20,7 +20,7 @@ fun Test(a: Int = remember { 0 }, b: Int = SomeComposable(), c: Int = remember {
 @Composable
 fun Test(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Test)<rememb...>,<SomeCo...>,<rememb...>:Test.kt")
+  sourceInformation(%composer, "C(Test):Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -37,6 +37,7 @@ fun Test(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %default:
   }
   if (%dirty and 0b10010011 != 0b10010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<rememb...>,<SomeCo...>,<rememb...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         a = <block>{

diff --git a/...ransformTests/testIntrinsicRememberOfDefaultParameters_AfterComposable[useFir = true].txt b/...ransformTests/testIntrinsicRememberOfDefaultParameters_AfterComposable[useFir = true].txt
@@ -20,7 +20,7 @@ fun Test(a: Int = remember { 0 }, b: Int = SomeComposable(), c: Int = remember {
 @Composable
 fun Test(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Test)<rememb...>,<SomeCo...>,<rememb...>:Test.kt")
+  sourceInformation(%composer, "C(Test):Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -37,6 +37,7 @@ fun Test(a: Int, b: Int, c: Int, %composer: Composer?, %changed: Int, %default:
   }
   if (%dirty and 0b10010011 != 0b10010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<rememb...>,<SomeCo...>,<rememb...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0001 != 0) {
         a = <block>{

diff --git a/...erIntrinsicTransformTestsStrongSkipping/testRememberWithDefaultParams[useFir = false].txt b/...erIntrinsicTransformTestsStrongSkipping/testRememberWithDefaultParams[useFir = false].txt
@@ -18,7 +18,7 @@ import androidx.compose.runtime.*
 @Composable
 fun Icon(param: Int, defaultParam: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Icon)P(1)<curren...>,<rememb...>:Test.kt")
+  sourceInformation(%composer, "C(Icon)P(1)<rememb...>:Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -30,6 +30,7 @@ fun Icon(param: Int, defaultParam: Int, %composer: Composer?, %changed: Int, %de
   }
   if (%dirty and 0b00010011 != 0b00010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<curren...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0010 != 0) {
         defaultParam = LocalColor.<get-current>(%composer, 0b0110)

diff --git a/...berIntrinsicTransformTestsStrongSkipping/testRememberWithDefaultParams[useFir = true].txt b/...berIntrinsicTransformTestsStrongSkipping/testRememberWithDefaultParams[useFir = true].txt
@@ -18,7 +18,7 @@ import androidx.compose.runtime.*
 @Composable
 fun Icon(param: Int, defaultParam: Int, %composer: Composer?, %changed: Int, %default: Int) {
   %composer = %composer.startRestartGroup(<>)
-  sourceInformation(%composer, "C(Icon)P(1)<curren...>,<rememb...>:Test.kt")
+  sourceInformation(%composer, "C(Icon)P(1)<rememb...>:Test.kt")
   val %dirty = %changed
   if (%default and 0b0001 != 0) {
     %dirty = %dirty or 0b0110
@@ -30,6 +30,7 @@ fun Icon(param: Int, defaultParam: Int, %composer: Composer?, %changed: Int, %de
   }
   if (%dirty and 0b00010011 != 0b00010010 || !%composer.skipping) {
     %composer.startDefaults()
+    sourceInformation(%composer, "<curren...>")
     if (%changed and 0b0001 == 0 || %composer.defaultsInvalid) {
       if (%default and 0b0010 != 0) {
         defaultParam = LocalColor.<get-current>(%composer, 0b0110)

diff --git a/.../java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt b/.../java/androidx/compose/compiler/plugins/kotlin/lower/ComposableFunctionBodyTransformer.kt
@@ -1299,54 +1299,53 @@ class ComposableFunctionBodyTransformer(
 
         val setDefaults = mutableStatementContainer()
         val skipDefaults = mutableStatementContainer()
-//        val parametersScope = Scope.ParametersScope()
-        parameters.fastForEachIndexed { slotIndex, param ->
-            val defaultIndex = scope.defaultIndexForSlotIndex(slotIndex)
-            val defaultValue = param.defaultValue?.expression
-            if (defaultParam != null && defaultValue != null) {
-//                val transformedDefault = inScope(parametersScope) {
-//                    defaultValue.expression.transform(this, null)
-//                }
-
-                // we want to call this on the transformed version.
-                defaultExprIsStatic[slotIndex] = defaultValue.isStatic()
-                defaultExpr[slotIndex] = defaultValue
-                val hasStaticDefaultExpr = defaultExprIsStatic[slotIndex]
-                when {
-                    isSkippableDeclaration && !hasStaticDefaultExpr &&
-                        dirty is IrChangedBitMaskVariable -> {
-                        // If we are setting the parameter to the default expression and
-                        // running the default expression again, and the expression isn't
-                        // provably static, we can't be certain that the dirty value of
-                        // SAME is going to be valid. We must mark it as UNCERTAIN. In order
-                        // to avoid slot-table misalignment issues, we must mark it as
-                        // UNCERTAIN even when we skip the defaults, so that any child
-                        // function receives UNCERTAIN vs SAME/DIFFERENT deterministically.
-                        setDefaults.statements.add(
-                            irIf(
-                                condition = irGetBit(defaultParam, defaultIndex),
-                                body = irBlock(
-                                    statements = listOf(
-                                        irSet(param, defaultValue),
-                                        dirty.irSetSlotUncertain(slotIndex)
+
+        withScope(defaultScope) {
+            parameters.fastForEachIndexed { slotIndex, param ->
+                val defaultIndex = scope.defaultIndexForSlotIndex(slotIndex)
+                val defaultValue = param.defaultValue?.expression
+                if (defaultParam != null && defaultValue != null) {
+
+                    // we want to call this on the transformed version.
+                    defaultExprIsStatic[slotIndex] = defaultValue.isStatic()
+                    defaultExpr[slotIndex] = defaultValue
+                    val hasStaticDefaultExpr = defaultExprIsStatic[slotIndex]
+                    when {
+                        isSkippableDeclaration && !hasStaticDefaultExpr &&
+                                dirty is IrChangedBitMaskVariable -> {
+                            // If we are setting the parameter to the default expression and
+                            // running the default expression again, and the expression isn't
+                            // provably static, we can't be certain that the dirty value of
+                            // SAME is going to be valid. We must mark it as UNCERTAIN. In order
+                            // to avoid slot-table misalignment issues, we must mark it as
+                            // UNCERTAIN even when we skip the defaults, so that any child
+                            // function receives UNCERTAIN vs SAME/DIFFERENT deterministically.
+                            setDefaults.statements.add(
+                                irIf(
+                                    condition = irGetBit(defaultParam, defaultIndex),
+                                    body = irBlock(
+                                        statements = listOf(
+                                            irSet(param, defaultValue),
+                                            dirty.irSetSlotUncertain(slotIndex)
+                                        )
                                     )
                                 )
                             )
-                        )
-                        skipDefaults.statements.add(
-                            irIf(
-                                condition = irGetBit(defaultParam, defaultIndex),
-                                body = dirty.irSetSlotUncertain(slotIndex)
+                            skipDefaults.statements.add(
+                                irIf(
+                                    condition = irGetBit(defaultParam, defaultIndex),
+                                    body = dirty.irSetSlotUncertain(slotIndex)
+                                )
                             )
-                        )
-                    }
-                    else -> {
-                        setDefaults.statements.add(
-                            irIf(
-                                condition = irGetBit(defaultParam, defaultIndex),
-                                body = irSet(param, defaultValue)
+                        }
+                        else -> {
+                            setDefaults.statements.add(
+                                irIf(
+                                    condition = irGetBit(defaultParam, defaultIndex),
+                                    body = irSet(param, defaultValue)
+                                )
                             )
-                        )
+                        }
                     }
                 }
             }
@@ -1565,7 +1564,7 @@ class ComposableFunctionBodyTransformer(
             // otherwise, we wrap the whole thing in an if expression with a skip
             scope.hasDefaultsGroup = true
             scope.metrics.recordGroup()
-            bodyPreamble.statements.add(irStartDefaults(sourceElement))
+            bodyPreamble.statements.add(irStartDefaults(sourceElement, defaultScope))
             bodyPreamble.statements.add(
                 irIfThenElse(
                     // this prevents us from re-executing the defaults if this function is getting
@@ -2086,12 +2085,15 @@ class ComposableFunctionBodyTransformer(
             irIfTraceInProgress(irCall(it))
         }
 
-    private fun irStartDefaults(element: IrElement): IrExpression {
-        return irMethodCall(
-            irCurrentComposer(),
-            startDefaultsFunction,
-            element.startOffset,
-            element.endOffset
+    private fun irStartDefaults(element: IrElement, scope: Scope.BlockScope): IrExpression {
+        return irWithSourceInformation(
+            irMethodCall(
+                irCurrentComposer(),
+                startDefaultsFunction,
+                element.startOffset,
+                element.endOffset
+            ),
+            scope
         )
     }
 

diff --git a/plugins/compose/compiler-hosted/testData/codegen/nestedLambda.ir.txt b/plugins/compose/compiler-hosted/testData/codegen/nestedLambda.ir.txt
@@ -237,7 +237,7 @@ MODULE_FRAGMENT
               key: CONST Int type=kotlin.Int value=-435887556
           CALL 'public final fun sourceInformation (composer: androidx.compose.runtime.Composer, sourceInformation: kotlin.String): kotlin.Unit declared in androidx.compose.runtime' type=kotlin.Unit origin=null
             composer: GET_VAR '$composer: androidx.compose.runtime.Composer? [assignable] declared in home.DynamicThemePrimaryColorsFromImage' type=androidx.compose.runtime.Composer? origin=null
-            sourceInformation: CONST String type=kotlin.String value="C(DynamicThemePrimaryColorsFromImage)P(1)21@595L28:main.kt#1wrmn"
+            sourceInformation: CONST String type=kotlin.String value="C(DynamicThemePrimaryColorsFromImage)P(1):main.kt#1wrmn"
         VAR name:$dirty type:kotlin.Int [val]
           GET_VAR '$changed: kotlin.Int declared in home.DynamicThemePrimaryColorsFromImage' type=kotlin.Int origin=null
         WHEN type=kotlin.Unit origin=IF
@@ -257,8 +257,12 @@ MODULE_FRAGMENT
                   $this: CALL 'public abstract fun <get-skipping> (): kotlin.Boolean declared in androidx.compose.runtime.Composer' type=kotlin.Boolean origin=null
                     $this: GET_VAR '$composer: androidx.compose.runtime.Composer? [assignable] declared in home.DynamicThemePrimaryColorsFromImage' type=androidx.compose.runtime.Composer? origin=null
             then: BLOCK type=kotlin.Unit origin=null
-              CALL 'public abstract fun startDefaults (): kotlin.Unit declared in androidx.compose.runtime.Composer' type=kotlin.Unit origin=null
-                $this: GET_VAR '$composer: androidx.compose.runtime.Composer? [assignable] declared in home.DynamicThemePrimaryColorsFromImage' type=androidx.compose.runtime.Composer? origin=null
+              BLOCK type=kotlin.Unit origin=null
+                CALL 'public abstract fun startDefaults (): kotlin.Unit declared in androidx.compose.runtime.Composer' type=kotlin.Unit origin=null
+                  $this: GET_VAR '$composer: androidx.compose.runtime.Composer? [assignable] declared in home.DynamicThemePrimaryColorsFromImage' type=androidx.compose.runtime.Composer? origin=null
+                CALL 'public final fun sourceInformation (composer: androidx.compose.runtime.Composer, sourceInformation: kotlin.String): kotlin.Unit declared in androidx.compose.runtime' type=kotlin.Unit origin=null
+                  composer: GET_VAR '$composer: androidx.compose.runtime.Composer? [assignable] declared in home.DynamicThemePrimaryColorsFromImage' type=androidx.compose.runtime.Composer? origin=null
+                  sourceInformation: CONST String type=kotlin.String value="21@595L28"
               WHEN type=kotlin.Unit origin=IF
                 BRANCH
                   if: WHEN type=kotlin.Boolean origin=OROR