Status Update
No update yet.
Comments
ku...@google.com
How about making OrElse return nullable?
public inline fun getInt(key: String): Int = getIntOrNull(key) ?: error("...")
public inline fun getIntOrElse(key: String, defaultValue: () -> Int?): Int? =
getIntOrNull(key) ?: defaultValue()
@PublishedApi
internal fun getIntOrNull(key: String): Int? {
...
}
ku...@google.com
Another alternative: return nullable or non-null depending on defaultValue, but it seems that the compiler can't keep up and we get Nothing? instead of Int? when defaultValue is a lambda and returns null:
public inline fun <T: Int?> getIntOrElse(key: String, defaultValue: () -> T): T
ku...@google.com
Or a variation to 2b: Only keep get*OrNull(), as users can do whatever they like with an elvis operator.
Some more reasons to embrace null:
- The signature of
kotlin.collections.Map.getalways returnsa nullable - Our non-Android SavedState is just a container of
MutableMap<String, Any?> Bundle.get*()on reference types return@Nullablevalues.
ko...@jetbrains.com
Both 2a/2b options seem god. 2b is more clear but requires changes on CMP side.
If it is released soon, we are ready to adopt it before we release the beta navigation.
Note: Kotlin collections have a nullable return type in get methods. This is more idiomatic
mg...@google.com
After consulting with Ian, we have decided to proceed with option 2b.
The concern with get*OrElse is that it could lead to unexpected behavior if putNull was previously called, returning the else() value instead of null. To avoid this, we will maintain a clear distinction between two approaches:
- Non-null data types, where
put*andget*are used. - Nullable data types, where
putNullorput*can be used, and values are retrieved withgetOrNull().
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Combine keyNotFoundError and valueNotFoundError
Expand for full commit details
Combine `keyNotFoundError` and `valueNotFoundError`
RelNote: "N/A"
Bug: 399820614
Fixes: 399818282
Test: SavedStateCodecTest
Test: SavedStateTest
Test: SavedStateAndroidTest
Change-Id: I518f38e4caa3063cb1e38fd763db5d030580f338
Files:
- M
savedstate/savedstate/api/restricted_current.txt - M
savedstate/savedstate/bcv/native/current.txt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt - M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateAndroidTest.android.kt - M
savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt - M
savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateCodecTest.kt - M
savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateTest.kt - M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
Hash: 40ca7f18520404f04bfded6477a45685fc157078
Date: Tue Mar 04 00:02:23 2025
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Fix missing get*OrElse in navigation.common
Expand for full commit details
Fix missing `get*OrElse` in `navigation.common`
- Pre-submit runs against ToT (max_version).
- The new SavedState does not include a `get*OrElse`, causing navigation to break.
Bug: 399820614
Test: N/A
Change-Id: Ife25463e2881342df4eae4221e9ef59b1e19d095
Files:
- M
navigation/navigation-common/build.gradle - M
navigation/navigation-runtime/src/androidMain/kotlin/androidx/navigation/NavController.android.kt
Hash: d62bb24b2575eb79280e67746e8c1ceb5a9d53c6
Date: Tue Mar 04 14:54:20 2025
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Add getOrNull in SavedStateReader
Expand for full commit details
Add `getOrNull` in `SavedStateReader`
- Introduces a `getOrNull` variant behaves as:
a. Key exists & type match == returns value.
b. Key doesn't exist == returns null.
c. Value is null (putNull) == returns null.
- Please note that `getOrNull` for primitives will cause auto-boxing.
RelNote: "Add `getOrNull` alternative methods for `SavedStateReader.get` variants."
Bug: 399820614
Test: SavedStateTest
Test: SavedStateAndroidTest
Change-Id: I6228c173c45e6137de7d4885550285de5e50ae15
Files:
- M
savedstate/savedstate/api/current.txt - M
savedstate/savedstate/api/restricted_current.txt - M
savedstate/savedstate/bcv/native/current.txt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt - M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateAndroidTest.android.kt - M
savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt - M
savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateTest.kt - M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
Hash: cd71b1c721a8cf9733aed3bea5361f528011641c
Date: Mon Mar 03 23:39:02 2025
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Remove inline from SavedState
Expand for full commit details
Remove `inline` from `SavedState`
- The changes in aosp/3500556 increased the amount of inlined code, raising concerns about excessive inlining in client code.
- Benchmarking showed no significant performance difference between using `inline` and non-`inline` method calls.
- Based on these findings, we decided to remove `inline` from `SavedStateReader` to reduce inlining and allow logic sharing without relying on `@PublishedApi`, which remains public for backward compatibility.
RelNote: "Remove `inline` from `SavedStateReader` and `SavedStateWriter` methods."
Bug: 399820614
Test: SavedStateCodecTest
Change-Id: If2a0264a865f5d1d0a1298903c83fa02cbd6a99e
Files:
- M
savedstate/savedstate/api/current.txt - M
savedstate/savedstate/api/restricted_current.txt - M
savedstate/savedstate/bcv/native/current.txt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateWriter.android.kt - M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateCodecAndroidTest.android.kt - M
savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt - M
savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateWriter.kt - M
savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateCodecTest.kt - M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt - M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateWriter.nonAndroid.kt
Hash: f09d0e201cae0bbbc77e28b0ecf6245425d65da6
Date: Mon Mar 03 23:47:41 2025
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Remove getOrElse in SavedStateReader
Expand for full commit details
Remove `getOrElse` in `SavedStateReader`
- Because `getOrNull()` is now available, users can achieve the same behavior with `getOrNull() ?: else()`, allowing us to reduce the public API surface and the number of inline methods (as `getOrElse` takes a lambda, it cannot be removed).
RelNote: "Remove `getOrElse` from `SavedStateReader` in favor of `getOrNull() ?: else()`."
Bug: 399820614
Test: SavedStateTest
Change-Id: I87317b0eaa0f6d0fad334cc3f81bfcc8a90e6313
Files:
- M
lifecycle/lifecycle-viewmodel-savedstate/src/commonMain/kotlin/androidx/lifecycle/SavedStateHandleSupport.kt - M
savedstate/savedstate/api/current.txt - M
savedstate/savedstate/api/restricted_current.txt - M
savedstate/savedstate/bcv/native/current.txt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/Recreator.android.kt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt - M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateAndroidTest.android.kt - M
savedstate/savedstate/src/commonMain/kotlin/androidx/savedstate/SavedStateReader.kt - M
savedstate/savedstate/src/commonTest/kotlin/androidx/savedstate/SavedStateTest.kt - M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
Hash: e04b610835745492aa66aa45be1426d57c1401b6
Date: Mon Mar 03 23:42:23 2025
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-main
Author: Marcello Galhardo <
Link:
Add non-reified get collections in SavedStateReader
Expand for full commit details
Add non-reified `get` collections in `SavedStateReader`
- Previously, retrieving a typed collection required a `reified` method, restricting its usage at runtime.
- Introducing a non-reified variant allows clients to call it with a dynamic type.
- This change also enables code encapsulation by reducing inlined code, with the reified method simply delegating to the non-reified one.
RelNote: "Add non-reified methods for `get` collections in `SavedStateReader`."
Bug: 399820614
Test: N/A
Change-Id: I0b641cba421761ee41bb8dc386cbc5db7cb67066
Files:
- M
savedstate/savedstate/api/current.txt - M
savedstate/savedstate/api/restricted_current.txt - M
savedstate/savedstate/src/androidMain/kotlin/androidx/savedstate/SavedStateReader.android.kt - M
savedstate/savedstate/src/androidUnitTest/kotlin/androidx/savedstate/SavedStateAndroidTest.android.kt - M
savedstate/savedstate/src/nonAndroidMain/kotlin/androidx/savedstate/SavedStateReader.nonAndroid.kt
Hash: 481a2c575f9c2c3a71e99cf61f093d079d69e1bc
Date: Tue Mar 04 10:55:25 2025
ap...@google.com
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523155]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
- androidx.savedstate.SavedStateWriter
[Gerrit:https://android-review.googlesource.com/3523153]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523152]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
ap...@google.com
Android API Change
This bug was referenced by a recent CL that changed the Android API surface area.
The
We'll wait until you mark this bug as 'Fixed' before starting our review, but please reach out if you'd like us to review it sooner.
Changes to savedstate/savedstate/api/current.txt
- androidx.savedstate
- androidx.savedstate.SavedStateReader
[Gerrit:https://android-review.googlesource.com/3523151]
[API-Approvers:
[Branch:androidx-main]
[LIBRARY_API_REVIEW_TAG:savedstate/savedstate/api/current.txt]
mg...@google.com
na...@google.com
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.lifecycle:lifecycle-viewmodel-savedstate:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-android:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-desktop:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-iosarm64:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-iossimulatorarm64:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-iosx64:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-linuxarm64:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-linuxx64:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-macosarm64:2.9.0-alpha12androidx.lifecycle:lifecycle-viewmodel-savedstate-macosx64:2.9.0-alpha12androidx.navigation:navigation-common:2.9.0-alpha08androidx.navigation:navigation-common-android:2.9.0-alpha08androidx.navigation:navigation-common-desktop:2.9.0-alpha08androidx.navigation:navigation-common-iosarm64:2.9.0-alpha08androidx.navigation:navigation-common-iossimulatorarm64:2.9.0-alpha08androidx.navigation:navigation-common-iosx64:2.9.0-alpha08androidx.navigation:navigation-common-linuxarm64:2.9.0-alpha08androidx.navigation:navigation-common-linuxx64:2.9.0-alpha08androidx.navigation:navigation-common-macosarm64:2.9.0-alpha08androidx.navigation:navigation-common-macosx64:2.9.0-alpha08androidx.navigation:navigation-runtime:2.9.0-alpha08androidx.navigation:navigation-runtime-android:2.9.0-alpha08androidx.navigation:navigation-runtime-desktop:2.9.0-alpha08androidx.navigation:navigation-runtime-iosarm64:2.9.0-alpha08androidx.navigation:navigation-runtime-iossimulatorarm64:2.9.0-alpha08androidx.navigation:navigation-runtime-iosx64:2.9.0-alpha08androidx.navigation:navigation-runtime-linuxarm64:2.9.0-alpha08androidx.navigation:navigation-runtime-linuxx64:2.9.0-alpha08androidx.navigation:navigation-runtime-macosarm64:2.9.0-alpha08androidx.navigation:navigation-runtime-macosx64:2.9.0-alpha08androidx.savedstate:savedstate:1.3.0-alpha10androidx.savedstate:savedstate-android:1.3.0-alpha10androidx.savedstate:savedstate-desktop:1.3.0-alpha10androidx.savedstate:savedstate-iosx64:1.3.0-alpha10androidx.savedstate:savedstate-linuxarm64:1.3.0-alpha10androidx.savedstate:savedstate-linuxx64:1.3.0-alpha10androidx.savedstate:savedstate-macosarm64:1.3.0-alpha10androidx.savedstate:savedstate-macosx64:1.3.0-alpha10
Description
JetBrains has asked if we could support a
get*OrNull()variant for our methods. The currentget*OrElse()does not allow returning null, and checking isNull beforehand is both unnecessary and inefficient.With AOSP/3500556 change to reduce excessive inlining, we plan to move from inline to non-inline functions, as benchmarks show minimal performance impact. This works well for
get*functions, butget*OrElse()would allocate lambdas if we don't use inline, which we don't want to.Here are the options before the beta:
Option 1: Do not support
get*OrNull().isNullandcontainsbefore agetruns at 60ns instead of 20ns and (2) wouldn’t be able to remove inline fromget*OrElse()due to lambda allocations.Option 2a: Support
get*OrNull(), which also allows migratingget*OrElse()out of inline functions.get*OrElse()since users can simply callgetOrNull() ?: else().get*OrElse(), so this would require extra work on their side.getOrNull()seems useful, andBundlealways boxes primitives anyway, since everything is stored in aMap<K, V>.Edit 03.03.25:
get*(), and change to return a@Nullablevalue.getfunction (Map). (2) Reduces API surface by half.SavedStateis a specializedMap, not a general-purpose one, so if anACTION_FOOexpectsEXTRA_FOO_ARG1andEXTRA_FOO_ARG2, their absence means execution can't proceed. Throwing an error for a missing key is (arguably) reasonable, whileget*OrNull()/get*OrElse()provide an explicit opt-out mechanism for cases where a null or default value is acceptable.