Skip to content

Partial stabilization of once_cell#105587

Merged
bors merged 3 commits intorust-lang:masterfrom
tgross35:once-cell-min
Mar 30, 2023
Merged

Partial stabilization of once_cell#105587
bors merged 3 commits intorust-lang:masterfrom
tgross35:once-cell-min

Conversation

@tgross35
Copy link
Contributor

@tgross35 tgross35 commented Dec 12, 2022

This PR aims to stabilize a portion of the once_cell feature:

  • core::cell::OnceCell
  • std::cell::OnceCell (re-export of the above)
  • std::sync::OnceLock

This will leave LazyCell and LazyLock unstabilized, which have been moved to the lazy_cell feature flag.

Tracking issue: #74465 (does not fully close, but it may make sense to move to a new issue)

Future steps for separate PRs:

To be stabilized API summary

// core::cell (in core/cell/once.rs)

pub struct OnceCell<T> { .. }

impl<T> OnceCell<T> {
    pub const fn new() -> OnceCell<T>;
    pub fn get(&self) -> Option<&T>;
    pub fn get_mut(&mut self) -> Option<&mut T>;
    pub fn set(&self, value: T) -> Result<(), T>;
    pub fn get_or_init<F>(&self, f: F) -> &T where F: FnOnce() -> T;
    pub fn into_inner(self) -> Option<T>;
    pub fn take(&mut self) -> Option<T>;
}

impl<T: Clone> Clone for OnceCell<T>;
impl<T: Debug> Debug for OnceCell<T>
impl<T> Default for OnceCell<T>;
impl<T> From<T> for OnceCell<T>;
impl<T: PartialEq> PartialEq for OnceCell<T>;
impl<T: Eq> Eq for OnceCell<T>;
// std::sync (in std/sync/once_lock.rs)

impl<T> OnceLock<T> {
    pub const fn new() -> OnceLock<T>;
    pub fn get(&self) -> Option<&T>;
    pub fn get_mut(&mut self) -> Option<&mut T>;
    pub fn set(&self, value: T) -> Result<(), T>;
    pub fn get_or_init<F>(&self, f: F) -> &T where F: FnOnce() -> T;
    pub fn into_inner(self) -> Option<T>;
    pub fn take(&mut self) -> Option<T>;
}

impl<T: Clone> Clone for OnceLock<T>;
impl<T: Debug> Debug for OnceLock<T>;
impl<T> Default for OnceLock<T>;
impl<#[may_dangle] T> Drop for OnceLock<T>;
impl<T> From<T> for OnceLock<T>;
impl<T: PartialEq> PartialEq for OnceLock<T>
impl<T: Eq> Eq for OnceLock<T>;
impl<T: RefUnwindSafe + UnwindSafe> RefUnwindSafe for OnceLock<T>;
unsafe impl<T: Send> Send for OnceLock<T>;
unsafe impl<T: Sync + Send> Sync for OnceLock<T>;
impl<T: UnwindSafe> UnwindSafe for OnceLock<T>;

No longer planned as part of this PR, and moved to the rust_cell_try feature gate:

impl<T> OnceCell<T> {
    pub fn get_or_try_init<F, E>(&self, f: F) -> Result<&T, E> where F: FnOnce() -> Result<T, E>;
}

impl<T> OnceLock<T> {
    pub fn get_or_try_init<F, E>(&self, f: F) -> Result<&T, E> where F: FnOnce() -> Result<T, E>;
}

I am new to this process so would appreciate mentorship wherever needed.

Post-merge update

With this merge, LazyCell and LazyLock have been moved under the gate lazy_cell (see #109736) and the get_or_try_init functions are under once_cell_try (see #109737)

@rustbot
Copy link
Collaborator

rustbot commented Dec 12, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 12, 2022
@tgross35 tgross35 marked this pull request as ready for review December 12, 2022 07:19
@rustbot
Copy link
Collaborator

rustbot commented Dec 12, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@tgross35
Copy link
Contributor Author

This needs FCP still

@rustbot modify labels: +T-libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 12, 2022
@tgross35 tgross35 mentioned this pull request Dec 12, 2022
9 tasks
@programmerjake
Copy link
Member

looks like you're stabilizing OnceLock, not LazyCell...the top comment should be fixed

@inquisitivecrystal inquisitivecrystal added relnotes Marks issues that should be documented in the release notes of the next release. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) labels Dec 12, 2022
@tgross35
Copy link
Contributor Author

Updated, thanks for the catch

@rustbot rustbot added the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Dec 12, 2022
@inquisitivecrystal inquisitivecrystal removed the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Dec 12, 2022
@klensy
Copy link
Contributor

klensy commented Dec 12, 2022

If this PR about stabilization, why slapping inline all over methods?

@rustbot rustbot added the A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) label Dec 12, 2022
@tgross35
Copy link
Contributor Author

tgross35 commented Dec 12, 2022

I just did not know if those were typically added at this time, since these functions are fairly good use cases. I have removed them, matklad suggested we can add them later

Edit: Moved this to #105651

@m-ou-se
Copy link
Member

m-ou-se commented Dec 30, 2022

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Dec 30, 2022

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@bors bors mentioned this pull request Mar 30, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8a7ca93): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.6% [-3.4%, -1.4%] 3
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.