Start using Stability.Runtime more broadly#5705
Conversation
Code Owners
|
c677b4d to
1f499e0
Compare
...sted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/AbstractComposeLowering.kt
Outdated
Show resolved
Hide resolved
ShikaSD
left a comment
There was a problem hiding this comment.
Great job here! A few top level comments:
- Let's extract interface
Unstable->Uncertainchange into a separate commit, it is an impactful change and having it separately will make it easier to review / revert. - Could you remind me why we cannot generate
static val $stableanymore? Can we only change it to getters forinternalclasses? I would love us to generate a minimal amount of code when possible considering code size concerns expressed by 1p.
.../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ClassStabilityTransformTests.kt
Show resolved
Hide resolved
.../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ClassStabilityTransformTests.kt
Show resolved
Hide resolved
....compiler.plugins.kotlin.ClassStabilityTransformTests/testComposableCall[useFir = false].txt
Outdated
Show resolved
Hide resolved
....compiler.plugins.kotlin.ClassStabilityTransformTests/testComposableCall[useFir = false].txt
Outdated
Show resolved
Hide resolved
...ilityTransformTests/testComposableCallWithUnstableFinalClassInSameModule[useFir = false].txt
Outdated
Show resolved
Hide resolved
...in.RememberIntrinsicTransformTests/testRememberInALoop_NoTrailingRemember[useFir = true].txt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...sted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/AbstractComposeLowering.kt
Outdated
Show resolved
Hide resolved
...sted/src/main/java/androidx/compose/compiler/plugins/kotlin/lower/AbstractComposeLowering.kt
Outdated
Show resolved
Hide resolved
1f499e0 to
c83c4ed
Compare
|
I’m using fixup commits to make this easier to review. I’ll squash them with |
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
51816b9 to
f5189cc
Compare
|
Could you please attach |
.../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ClassStabilityTransformTests.kt
Show resolved
Hide resolved
...tests/src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ComposeCrossModuleTests.kt
Show resolved
Hide resolved
.../src/jvmTest/kotlin/androidx/compose/compiler/plugins/kotlin/ClassStabilityTransformTests.kt
Outdated
Show resolved
Hide resolved
....compiler.plugins.kotlin.ClassStabilityTransformTests/testComposableCall[useFir = false].txt
Outdated
Show resolved
Hide resolved
....compiler.plugins.kotlin.ClassStabilityTransformTests/testComposableCall[useFir = false].txt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...expect for those explicitly marked as known stable Relnote: "Started inferring the stability of all interfaces to be `Stability.Unknown`, expect for those explicitly marked as known stable."
87cad39 to
a6bedbc
Compare
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Outdated
Show resolved
Hide resolved
d7162e7 to
a8201e6
Compare
...compiler-hosted/src/main/java/androidx/compose/compiler/plugins/kotlin/analysis/Stability.kt
Show resolved
Hide resolved
a8201e6 to
622c7e1
Compare
New changes affect files owned by this reviewer. Re-review required.
622c7e1 to
3c016e2
Compare
|
cc @Tapchicoma to approve integration tests |
There was a problem hiding this comment.
If you want to run the test with the latest max supported Gradle version - just use @GradleTestVersions(minVersion = TestVersions.Gradle.MAX_SUPPORTED
There was a problem hiding this comment.
The second build performed in this test uses a constant Kotlin version of 2.3.10
so if I use TestVersions.Gradle.MAX_SUPPORTED, I think this test could unexpectedly break if TestVersions.Gradle.MAX_SUPPORTED becomes incompatible with Kotlin 2.3.10.
...lin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/ComposeIT.kt
Outdated
Show resolved
Hide resolved
...lin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/ComposeIT.kt
Outdated
Show resolved
Hide resolved
Tapchicoma
left a comment
There was a problem hiding this comment.
It would be nice to update test
Relnote: "Started using `Stability.Runtime` more broadly. Now, when an element depends on the stability of an `internal` or `public` class defined in another file, the element will no longer infer the stability of that class and will depend on the runtime stability of that class instead." Bug: 427530633
On JVM, there is now different logic governing when runtime stability is used, so the code reading the known stable bit of stability bitmasks has become unreachable. On other platforms, the code reading that bit is also unreachable, because when the stability of a module is analyzed, any declarations encountered with origin `IR_EXTERNAL_DECLARATION_STUB` are guaranteed to be from a different module.
…tComposeLowering` This CL makes it so that a getter call is inferred as static only when it is an invocation of a default getter of a read-only property, and the call is made in the same file that the property is defined in. Bug: 427530633
3c016e2 to
254c7f0
Compare
New changes affect files owned by this reviewer. Re-review required.
This PR makes it so that if an element depends on the stability of an
internalorpublicclass defined in another file, the element will no longer infer the stability of that class and will depend on the runtime stability of that class instead.Bug: 427530633