Leverage &mut in OnceLock when possible by tisonkun · Pull Request #149469 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Comment on lines -411 to -412
| debug_assert!(self.is_initialized()); |
|---|
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already checked in get_unchecked
| if self.get_mut().is_none() { |
|---|
| self.initialize(f)?; |
| } |
| debug_assert!(self.is_initialized()); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already checked in get_unchecked_mut
| #[inline] |
| fn is_initialized(&self) -> bool { |
| fn initialized(&self) -> bool { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aligned with initialized_mut.
is_initialized_mut doesn't follow some idioms that is_xxx would typically take &self.
Signed-off-by: tison wander4096@gmail.com
Comment on lines +522 to +529
| fn initialized_mut(&mut self) -> bool { |
|---|
| // `state()` does not perform an atomic load, so prefer it over `is_complete()`. |
| let state = self.once.state(); |
| match state { |
| OnceExclusiveState::Complete => true, |
| _ => false, |
| } |
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what LazyLock already does.
Failed to set assignee to yoshuawuyts: invalid assignee
Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.
Thanks for the ping @tisonkun, though I don't think I'm the right person to review this PR.
Thanks for the ping @tisonkun, though I don't think I'm the right person to review this PR.
Yeah .. Do you know where to find the right person? I remember you as an experienced async Rust developer and consider that you can give a review on whether the changes are reasonable.
rustbot auto selects @Mark-Simulacrum but IIRC he is busy on Rust releases and builds. Thus I hope we can expand the bandwidth with other proper reviewer.
I continue to actively review PRs, typically on a weekly basis. Please be patient with our reviewer pool.
Labels
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.