Skip to content
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 OnceCell::with_value and improve Clone implementation #170

Merged
merged 5 commits into from Mar 3, 2022

Conversation

a1phyr
Copy link
Contributor

@a1phyr a1phyr commented Feb 5, 2022

This PR adds OnceCell::with_value to both sync::OnceCell and unsync::OnceCell. This enables creating an initialized OnceCell at compile time and/or without synchronization cost.

Additionally, this improve Clone implementations by removing a panicking branch, reducing synchronization cost and overriding clone_from implementation.

Closes #164

@matklad
Copy link
Owner

@matklad matklad commented Mar 3, 2022

Hm, I am a bit torn here -- it feels like From is probably the best interface here. But that can't be const. On the other hand, const-creating a full OnceCell feels a bit odd. Do we have a real use-case for that? At the same time, the API does feel rather tidy.

🤔 yeah, I guess, lets have this! The implementation looks good to me, but lets add a couple of tests to https://github.com/matklad/once_cell/blob/master/tests/it.rs to make sure the new API doesn't get broken by accident.

@a1phyr
Copy link
Contributor Author

@a1phyr a1phyr commented Mar 3, 2022

Actually, I didn't see the From implementation when I wrote this patch, so I wrote with_value for that, so I don't have a strong will to add this.

There might be a use-case for something like that, I guess (though very specific):

const fn get_some_value() -> Option<T> {
    /* ... */
}

static CELL: OnceCell<T> = match get_some_value() {
    Some(value) => OnceCell::with_value(value),
    None => OnceCell::new(),
};

I think the main benefit of this would be discoverability, which is not great today, as shown by the linked issue (and my own experience ^^').

Copy link
Owner

@matklad matklad left a comment

bors r+

Thanks!

@bors
Copy link
Contributor

@bors bors bot commented Mar 3, 2022

Build succeeded:

@bors bors bot merged commit 090caea into matklad:master Mar 3, 2022
2 checks passed
@a1phyr a1phyr deleted the with_value branch Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants