Skip to content

add a take method which takes a mutable reference - #100

Merged
matklad merged 1 commit into
matklad:masterfrom
sunshowers:reset
May 13, 2020
Merged

matklad merged 1 commit into
matklad:masterfrom
sunshowers:reset

Conversation

@sunshowers

Copy link
Copy Markdown
Contributor

I use OnceCell extensively in my code to cache lazily computed data on
structs. Sometimes, I mutate the data -- in which case I have to invalidate
those caches.

This is of course possible to do by oneself, but it's slightly non-obvious
that the "once" bit of OnceCell only applies in the shared reference context.

@sunshowers
sunshowers force-pushed the reset branch 3 times, most recently from c705ac2 to 603162d Compare May 4, 2020 22:22
@matklad

matklad commented May 5, 2020

Copy link
Copy Markdown
Owner

Hm, I am a bit on the fence about this... My first reaction is that mem::take should be good enough. My second thought is that Option::take is a thing....

What do you think about renaming reset to take?

@sunshowers

Copy link
Copy Markdown
Contributor Author

Sure, sounds reasonable to me for symmetry.

I use `OnceCell` extensively in my code to cache lazily computed data on
structs. Sometimes, I mutate the data -- in which case I have to invalidate
those caches.

This is of course possible to do by oneself, but it's slightly non-obvious
that the "once" bit of OnceCell only applies in the shared reference context.
@sunshowers sunshowers changed the title [impl] add a reset method which takes a mutable reference add a take method which takes a mutable reference May 5, 2020
@sunshowers

Copy link
Copy Markdown
Contributor Author

done, thanks!

@sunshowers

Copy link
Copy Markdown
Contributor Author

ah, looks like mem::take isn't on the MSRV 1.31.1. think the MSRV should be bumped to 1.40?

@matklad

matklad commented May 13, 2020

Copy link
Copy Markdown
Owner

I'd prefer not to bump MSRV unless necessary (really, I'd prefer to not bump MSRV at all, and just implement rust-lang/rfcs#2788). mem::take is just mem::replace(_, Default::default), so should not be hard to poly-fill.

@matklad

matklad commented May 13, 2020

Copy link
Copy Markdown
Owner

I'll fix this up myself, thanks for sending a PR and sorry for this taking so long to review :)

@matklad
matklad merged commit 8a322d0 into matklad:master May 13, 2020
@matklad

matklad commented May 13, 2020

Copy link
Copy Markdown
Owner

Also, wow, this is the hundredth issue/pr... Didn't expect that from a relatively simple utility crate :)

@matklad

matklad commented May 13, 2020

Copy link
Copy Markdown
Owner

Published as 1.4.0: https://crates.io/crates/once_cell/1.4.0

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