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.

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

Add UnwindSafe to Once #90625

Merged
merged 2 commits into from
Dec 23, 2021
Merged

Add UnwindSafe to Once #90625

merged 2 commits into from
Dec 23, 2021

Conversation

Milo123459
Copy link
Contributor

Fixes #43469

@rust-highfive
Copy link
Collaborator

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

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 5, 2021
@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Nov 11, 2021
@scottmcm scottmcm added needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. and removed needs-rfc This change is large or controversial enough that it should have an RFC accepted before doing it. labels Nov 13, 2021
@camelid
Copy link
Member

camelid commented Dec 10, 2021

r? rust-lang/libs

@camelid

This comment has been minimized.

Sign in to view
@dtolnay dtolnay removed the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 10, 2021
dtolnay
dtolnay requested changes Dec 10, 2021
View reviewed changes
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

This needs a manual RefUnwindSafe impl as well.

@dtolnay dtolnay assigned dtolnay and unassigned Mark-Simulacrum Dec 10, 2021
@dtolnay
Copy link
Member

dtolnay commented Dec 10, 2021

@rust-lang/libs-api:
@rfcbot fcp merge

All of the Once methods behave atomically and there is no possibility of the Once being left in a corrupted state following an unwind. UnwindSafe and RefUnwindSafe auto trait impls only didn't kick in because of the presence of a Cell inside Waiter.

@rfcbot
Copy link

rfcbot commented Dec 10, 2021 •
edited by BurntSushi
Loading

Team member @dtolnay 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.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Dec 10, 2021
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Dec 13, 2021
@rfcbot
Copy link

rfcbot commented Dec 13, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Dec 13, 2021
@dtolnay
Copy link
Member

dtolnay commented Dec 20, 2021

  • Squashed, and added RefUnwindSafe as requested above (and implied by the branch name)

@dtolnay dtolnay added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2021
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Dec 23, 2021
@rfcbot
Copy link

rfcbot commented Dec 23, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@dtolnay
Copy link
Member

dtolnay commented Dec 23, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Dec 23, 2021

📌 Commit 91161ed has been approved by dtolnay

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 23, 2021
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 23, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 23, 2021
@bors
Auto merge of rust-lang#92232 - matthiaskrgr:rollup-eqdac7z, r=matthi…
c09a952 
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#90625 (Add `UnwindSafe` to `Once`)
 - rust-lang#92121 (disable test with self-referential generator on Miri)
 - rust-lang#92166 (Fixed a small typo in ui test comments)
 - rust-lang#92203 (Store a `DefId` instead of an `AdtDef` in `AggregateKind::Adt`)
 - rust-lang#92231 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 40c6720 into rust-lang:master Dec 23, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 23, 2021
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camelid camelid camelid left review comments

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

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. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.59.0
Development

Successfully merging this pull request may close these issues.

std::sync::Once: Should Once be UnwindSafe or RefUnwindSafe?