Skip to content

Simplify the unwind crate#159010

Open
bjorn3 wants to merge 2 commits into
rust-lang:mainfrom
bjorn3:refactor_unwind
Open

Simplify the unwind crate#159010
bjorn3 wants to merge 2 commits into
rust-lang:mainfrom
bjorn3:refactor_unwind

Conversation

@bjorn3

@bjorn3 bjorn3 commented Jul 9, 2026

Copy link
Copy Markdown
Member

Use cfg_select! and unify type definitions.

@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 Jul 9, 2026
@rustbot

rustbot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from 6 candidates

@bjorn3

bjorn3 commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

@xobs would you be able to check if the second commit doesn't break Xous?

@rust-log-analyzer

This comment has been minimized.

@bjorn3 bjorn3 force-pushed the refactor_unwind branch from 3581b47 to 4866a9c Compare July 9, 2026 14:46
@rustbot

rustbot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

miri is developed in its own repository. If possible, consider making this change to rust-lang/miri instead.

cc @rust-lang/miri


#[cfg(any(target_arch = "loongarch32", target_arch = "loongarch64"))]
pub const unwinder_private_data_size: usize = 2;
const unwinder_private_data_size: usize = cfg_select! {

@RalfJung RalfJung Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given that this is apparently mentioned in a public type, shouldn't this be public?

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Users don't need to care about the exact value. They just need to produce an array of the correct length that the unwinder can later fill with whatever data it needs, which can be done using [0; _]. The size of this array is just an implementation detail of the unwinder that happens to be part of the ABI.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's apparently part of the public API/ABI so I see no benefit to making it private here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

llvm-libunwind doesn't consider private to be part of the public api. It has a bunch of private_* and reserved fields in the place with varying sizes and without any unwinder_private_data_size const: https://github.com/llvm/llvm-project/blob/06499c927e5012166f4553c58f8af593521ea14d/libunwind/include/unwind_itanium.h#L25-L37 _Unwind_Exception isn't meant to be constructed using _Unwind_Exception { field1, field2, field3 }, but rather by allocating memory for the exception object and then initializing select fields. I could change panic_unwind to do that too and then make the field private.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants