Skip to content

move once module out of poison #147125

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 21, 2025
Merged

move once module out of poison #147125

merged 4 commits into from
Oct 21, 2025

Conversation

connortsui20
Copy link
Contributor

@connortsui20 connortsui20 commented Sep 28, 2025 •
edited
Loading

From #134645 (comment), since Once will not have a non-poisoning variant, we remove it from the poison module.

Additionally:

  1. Renames once::ExclusiveState to OnceExclusiveState since it was a bit confusing reading just ExclusiveState where it is used.
  2. Reorders a few module definitions and re-exports in library/std/src/sync/mod.rs for clarity.

Also, once this is merged, I think that we can begin the process of stabilizing sync_poison_mod

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 28, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
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

@connortsui20
Copy link
Contributor Author

r? @tgross35

@rustbot rustbot assigned tgross35 and unassigned ibraheemdev Oct 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2025

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

tgross35
tgross35 reviewed Oct 16, 2025
View reviewed changes
Copy link
Contributor

@tgross35 tgross35 left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits here then r=me

Additionally:

1. Renames `once::ExclusiveState` to `OnceExclusiveState` since it was a bit confusing reading just `ExclusiveState` where it is used.
2. Reorders a few module definitions and re-exports in `library/std/src/sync/mod.rs` for clarity.

For future reference, since these and the original Once change are all orthogonal, they'd ideally be split to three separate commits. It's a separation of concerns (cleanup vs. behavior change) and makes the diffs easier to follow.

View changes since this review

library/std/src/sync/mod.rs
Condvar,
Once, OnceState,
RwLock, RwLockReadGuard, RwLockWriteGuard,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like alphabetical ordering may have gotten flipped between Mutex and TryLockError. (would be nice if we could just remove the rustfmt::skip from this module but it does make things messy)

Copy link
Contributor Author

@connortsui20 connortsui20 Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually did this purposefully (though Condvar should probably come after RwLock now that I think about it because RwLock relies on TryLockError too). Once I finish revisions let me know if the ordering makes sense (commit 7b61403)

@tgross35
Copy link
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@connortsui20
rename once::ExclusiveState to OnceExclusiveState
a61c8be 
It is a bit confusing when reading code that uses this type since it is
not immediately obvious that it is specific to `Once`.

Signed-off-by: Connor Tsui <[email protected]>
@rustbot
Copy link
Collaborator

rustbot commented Oct 17, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@connortsui20
clean up some documentation
0758e19 
Signed-off-by: Connor Tsui <[email protected]>
@connortsui20
reorganize library/std/src/sync/mod.rs file
7b61403 
Moves things around to make a bit more sense (plus prepare moving `once`
out of `poison`.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20
move once module out of poison
3a9c521 
Since `Once` will not have a non-poisoning variant, we remove it from
the `poison` module.

Signed-off-by: Connor Tsui <[email protected]>
@connortsui20
Copy link
Contributor Author

connortsui20 commented Oct 17, 2025 •
edited
Loading

@rustbot ready

(Also sorry about the commit hygiene I think I was just being lazy)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 17, 2025
@tgross35
Copy link
Contributor

(Also sorry about the commit hygiene I think I was just being lazy)

No worries, we don't have any commit guidelines so it's just nice to have rather than required :). Thanks for doing it now, though!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 20, 2025

📌 Commit 3a9c521 has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 20, 2025
@matthiaskrgr
Rollup merge of rust-lang#147125 - connortsui20:poison-once-remove, r…
c48cf80 
…=tgross35

move `once` module out of `poison`

From rust-lang#134645 (comment), since `Once` will not have a non-poisoning variant, we remove it from the `poison` module.

Additionally:

1. Renames `once::ExclusiveState` to `OnceExclusiveState` since it was a bit confusing reading just `ExclusiveState` where it is used.
2. Reorders a few module definitions and re-exports in `library/std/src/sync/mod.rs` for clarity.

Also, once this is merged, I think that we can begin the process of stabilizing [`sync_poison_mod`](rust-lang#134646)
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 20, 2025
Closed
bors added a commit that referenced this pull request Oct 20, 2025
@bors
Auto merge of #147920 - matthiaskrgr:rollup-b3wqqf9, r=matthiaskrgr
4d014a5 
Rollup of 3 pull requests

Successful merges:

 - #147125 (move `once` module out of `poison`)
 - #147860 (rustdoc search: relax rules for identifiers)
 - #147910 (handle spurious returns of `wait_timeout` in test)

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar Zalathar mentioned this pull request Oct 21, 2025
Merged
bors added a commit that referenced this pull request Oct 21, 2025
@bors
Auto merge of #147928 - Zalathar:rollup-2lmpajs, r=Zalathar
e821cb8 
Rollup of 5 pull requests

Successful merges:

 - #147125 (move `once` module out of `poison`)
 - #147800 (Add `Cacheable` trait alias in `rustc_public_bridge`)
 - #147860 (rustdoc search: relax rules for identifiers)
 - #147916 (Update books)
 - #147924 (rustc-dev-guide subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 79e4669 into rust-lang:master Oct 21, 2025
11 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 21, 2025
rust-timer added a commit that referenced this pull request Oct 21, 2025
@rust-timer
Unrolled build for #147125
a213658 
Rollup merge of #147125 - connortsui20:poison-once-remove, r=tgross35

move `once` module out of `poison`

From #134645 (comment), since `Once` will not have a non-poisoning variant, we remove it from the `poison` module.

Additionally:

1. Renames `once::ExclusiveState` to `OnceExclusiveState` since it was a bit confusing reading just `ExclusiveState` where it is used.
2. Reorders a few module definitions and re-exports in `library/std/src/sync/mod.rs` for clarity.

Also, once this is merged, I think that we can begin the process of stabilizing [`sync_poison_mod`](#134646)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@tgross35 tgross35

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.92.0

Development

Successfully merging this pull request may close these issues.

5 participants

@connortsui20 @rustbot @tgross35 @bors @ibraheemdev