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

Upgrade parking_lot to 0.10 version and use MaybeUninit for it #91

Merged
merged 6 commits into from
Feb 4, 2020
Merged

Upgrade parking_lot to 0.10 version and use MaybeUninit for it #91

merged 6 commits into from
Feb 4, 2020

Conversation

tyranron
Copy link
Contributor

@tyranron tyranron commented Jan 27, 2020

This PR upgrades parking_lot dependency to 0.10 version.

Additionally

Makes OnceCell to use MaybeUninit under-the-hood when parking_lot feature is enabled

Checklist

  • Cargo.lock.min updated with cargo +nightly generate-lockfile -Z minimal-versions + cargo lock translate -v1.
  • CI job with minimal Rust version 1.36.0 for parking_lot feature added
  • rustfmted
  • CHANGELOG.md entry added

@matklad
Copy link
Owner

matklad commented Jan 27, 2020

While we are at it, it also makes sense to use MaybeUninit here:

pub(crate) value: UnsafeCell<Option<T>>,
. And, as this bumps MSRV, it should bump to 1.4.0.

.travis.yml Outdated Show resolved Hide resolved
@tyranron
Copy link
Contributor Author

@matklad

While we are at it, it also makes sense to use MaybeUninit here

Maybe as a separate PR?

@matklad
Copy link
Owner

matklad commented Jan 27, 2020

It makes sense to do this in the same PR where we bump MSRV, because that's exactly wha allows us ot use maybe uninit

@tyranron
Copy link
Contributor Author

@matklad also, what is the proper way for you to update Cargo.lock.min to V1 lock-file format?

@matklad
Copy link
Owner

matklad commented Jan 27, 2020

Don't know about v1, haven't looked into it yet. In general, it's best to just generate .min.lock using Cargo 1.31

@tyranron
Copy link
Contributor Author

Hmm... I've used cargo +nightly generate-lockfile -Z minimal-versions + cargo lock translate -v1 and it seems to work well.

@tyranron tyranron changed the title Upgrade parking_lot to 0.10 version WIP: Upgrade parking_lot to 0.10 version and use MaybeUninit Jan 28, 2020
@tyranron
Copy link
Contributor Author

@matklad I've landed the initial implementation via MaybeUninit for sync::OnceCell. This is majorly based on #72 (thank you, @pitdicker!).

As I have a little experience dealing with unsafes and low-level stuff like uninitialized memory, please make an early review of this. Once it's OK, I've land the similar changes for Lazy.

@pitdicker I'll be happy to see your comments too!

src/imp_pl.rs Outdated Show resolved Hide resolved
src/imp_pl.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
examples/bench_acquire.rs Show resolved Hide resolved
src/imp_pl.rs Show resolved Hide resolved
src/imp_pl.rs Show resolved Hide resolved
src/imp_std.rs Outdated Show resolved Hide resolved
src/imp_pl.rs Outdated Show resolved Hide resolved
@tyranron
Copy link
Contributor Author

@matklad what we have now:

  • MaybeUninit is used only for parking_lot feature;
  • MSRV is increased only when parking_lot feature is enabled;
  • get_unchecked, get_mut and into_inner accessors implementations moved into submodules (it turned out the most reasonable way to avoid unnecessary casts and inconsistencies);
  • all branch changes were splitted to separate meaningful commits.

@tyranron tyranron requested a review from matklad January 29, 2020 18:54
@tyranron tyranron changed the title WIP: Upgrade parking_lot to 0.10 version and use MaybeUninit Upgrade parking_lot to 0.10 version and use MaybeUninit Jan 29, 2020
@tyranron tyranron changed the title Upgrade parking_lot to 0.10 version and use MaybeUninit Upgrade parking_lot to 0.10 version and use MaybeUninit for it Jan 29, 2020
src/lib.rs Outdated Show resolved Hide resolved
src/imp_pl.rs Show resolved Hide resolved
src/imp_pl.rs Outdated Show resolved Hide resolved
@tyranron tyranron requested a review from matklad February 2, 2020 13:28
@matklad
Copy link
Owner

matklad commented Feb 4, 2020

bors r+

Let's land this now! I think there's a couple of tweaks I want to do, but I've also found an unrelated obscure bug which I'd love to fix, so I do the changes myself

bors bot added a commit that referenced this pull request Feb 4, 2020
91: Upgrade parking_lot to 0.10 version and use MaybeUninit for it r=matklad a=tyranron

This PR upgrades `parking_lot` dependency to `0.10` version.
- [Changelog](https://github.com/Amanieu/parking_lot/blob/0.10.0/CHANGELOG.md#parking_lot-0100-parking_lot_core-070-lock_api-032-2019-11-25)

### Additionally

Makes `OnceCell` to use `MaybeUninit` under-the-hood when `parking_lot` feature is enabled

### Checklist

- [x] `Cargo.lock.min` updated with `cargo +nightly generate-lockfile -Z minimal-versions` + `cargo lock translate -v1`.
- [x] ~~CI job with minimal Rust version `1.36.0` for `parking_lot` feature added~~
- [x] `rustfmt`ed
- [x] `CHANGELOG.md` entry added

Co-authored-by: tyranron <tyranron@gmail.com>
@bors
Copy link
Contributor

bors bot commented Feb 4, 2020

Build succeeded

@bors bors bot merged commit 0361fb3 into matklad:master Feb 4, 2020
@tyranron tyranron deleted the upgrade-parking-lot-to-0.10 branch February 4, 2020 17:49
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.

3 participants