-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add persistent Collection builder functions #166
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
fe58e7d
to
bc52615
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation for the functions should be amended. Otherwise, LGTM.
bc52615
to
20ad7a1
Compare
* The list passed as a receiver to the [builderAction] is valid only inside that function. | ||
* Using it outside the function produces an unspecified behavior. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builders in kotlinx.collections.immutable are capable of constructing a new persistent collection multiple times. However, I'm struggling to identify a compelling use case for retaining a reference to the builder and utilizing it outside the builderAction. If we constraint the builder to be valid only within the function, could using a MutableList
as the receiver be a more performance-efficient choice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @ilya-g, WDYT?
62dfb51
to
c90d590
Compare
c90d590
to
1ac235e
Compare
@OptIn(ExperimentalTypeInference::class, ExperimentalContracts::class) | ||
public inline fun <T> buildPersistentList(@BuilderInference builderAction: PersistentList.Builder<T>.() -> Unit): PersistentList<T> { | ||
contract { callsInPlace(builderAction, InvocationKind.EXACTLY_ONCE) } | ||
return persistentListOf<T>().builder().apply(builderAction).build() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Annotate
@BuilderInference
for better generic type support. - Add
InvocationKind.EXACTLY_ONCE
contract to indicate the function parameter will be invoked exactly one time.
1ac235e
to
f4b5f15
Compare
Closes #137.