Skip to content

Revert #123203 #125377

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
Closed
wants to merge 1 commit into from
Closed

Revert #123203 #125377

wants to merge 1 commit into from

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented May 21, 2024

This reverts #123203 which caused a regression by making Context no longer UnwindSafe.

It may be possible to re-merge #123203 once we move towards deprecating UnwindSafe entirely.

Fixes #125193
@Amanieu
Revert "Rollup merge of rust-lang#123203 - jkarneges:context-ext, r=A…
de691db 
…manieu" This reverts commit 9c1c0bf, reversing changes made to e41d7e7.
@rustbot
Copy link
Collaborator

rustbot commented May 21, 2024

r? @scottmcm

rustbot has assigned @scottmcm.
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
@rustbot rustbot added 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 May 21, 2024
@Amanieu Amanieu mentioned this pull request May 21, 2024
Closed
@matthiaskrgr
Copy link
Member

Could we add #125193 (comment) as a test?
@inquisitivecrystal
Copy link
Contributor

I do want to point out that there's a pretty simple alternative here: you could simply implement UnwindSafe and RefUnwindSafe for Context (or use AssertUnwindSafe). Whether or not this alternative is preferred is a judgement call, and it may be reasonable to revert pending discussion.
@workingjubilee
Copy link
Member

I think AssertUnwindSafe makes the most sense, pending a decision about what "unwind safety" even means, since it's just this one field.
@workingjubilee workingjubilee mentioned this pull request May 22, 2024
Merged
@apiraino apiraino removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 22, 2024
@Amanieu
Copy link
Member Author

Amanieu commented May 22, 2024

Closing in favor of #125392.
@Amanieu Amanieu closed this May 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 23, 2024
@matthiaskrgr
Rollup merge of rust-lang#125392 - workingjubilee:unwind-a-problem-in…
5126d4b 
…-context, r=Amanieu Wrap Context.ext in AssertUnwindSafe Fixes rust-lang#125193 Alternative to rust-lang#125377 Relevant to rust-lang#123392 I believe this approach is justifiable due to the fact that this function is unstable API and we have been considering trying to dispose of the notion of "unwind safety". Making a more long-term decision should be considered carefully as part of stabilizing `fn ext`, if ever. r? `@Amanieu`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 23, 2024
@rust-timer
Unrolled build for rust-lang#125392
c7c4c12 
Rollup merge of rust-lang#125392 - workingjubilee:unwind-a-problem-in-context, r=Amanieu Wrap Context.ext in AssertUnwindSafe Fixes rust-lang#125193 Alternative to rust-lang#125377 Relevant to rust-lang#123392 I believe this approach is justifiable due to the fact that this function is unstable API and we have been considering trying to dispose of the notion of "unwind safety". Making a more long-term decision should be considered carefully as part of stabilizing `fn ext`, if ever. r? `@Amanieu`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 24, 2024
@matthiaskrgr
Rollup merge of #125392 - workingjubilee:unwind-a-problem-in-context,…
789d269 
… r=Amanieu Wrap Context.ext in AssertUnwindSafe Fixes rust-lang/rust#125193 Alternative to rust-lang/rust#125377 Relevant to rust-lang/rust#123392 I believe this approach is justifiable due to the fact that this function is unstable API and we have been considering trying to dispose of the notion of "unwind safety". Making a more long-term decision should be considered carefully as part of stabilizing `fn ext`, if ever. r? `@Amanieu`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

8 participants

@Amanieu @rustbot @matthiaskrgr @inquisitivecrystal @workingjubilee @dtolnay @apiraino @scottmcm