Fix StabilityInferencer's handling of internal types
#5596
Conversation
derekxu16
requested review from
ShikaSD,
andrewbailey,
bentrengrove and
chuckjaz
as code owners
|
Could you also add a test to verify that the |
In the past, if an `internal` type `T` was inferred to be stable when it was compiled in a compilation unit `A`, we would infer it to be stable when compiling a compilation unit `B` that uses the type. This caused failures in situations in which `T` was later made unstable by an edit and `A` was incrementally recompiled, but `B` was not. This change eliminates the possibility of such failures. Test: ClassStabilityTransformTests.testInferredStabilityOfInternalTypesFromOtherCompilationUnits Fixes: 427530633
derekxu16
force-pushed
the
prr/derekx/fix-stability-inferencer
branch
from
278124e to
eb9c553
Compare
| """, | ||
| """ | ||
| @Composable | ||
| internal fun CanSkip(foo: Foo = Foo()) { |
There was a problem hiding this comment.
I am not sure this test makes a lot of sense as is.
You want to check for codegen of parameters with the types of internal class FooStable(val value: Int) and internal class FooUnstable(var value: Int) instead of unstable class only.
The other thing I noticed is that unstable class produced changedInstance rather than changed call, which tells me that changedInstance is always generated for Runtime stability. Please file a bug for this.
Also no need to check default arguments, that does not affect stability here.
In the past, if an
internaltypeTwas inferred to be stable when it was compiled in a compilation unitA, we would infer it to be stable when compiling a compilation unitBthat uses the type. This caused failures in situations in whichTwas later made unstable by an edit andAwas incrementally recompiled, butBwas not. This change eliminates the possibility of such failures.Test: ClassStabilityTransformTests.testInferredStabilityOfInternalTypesFromOtherCompilationUnits
Fixes: 427530633