Skip to content

refactors sus_panic and sus_unreachable #470

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

cjdb
Copy link
Contributor

@cjdb cjdb commented Apr 29, 2025

These macros have been changed to functions, which will provide better ergonomics for C++. Care was taken to ensure that calls to sus::panic weren't substantially worse than expanding sus_panic. In addition, sus_panic_with_message was refactored into sus::panic.

The same logic was applied to refactoring sus_unreachable, although the assembly wasn't inspected.

cjdb added 10 commits April 29, 2025 16:16
This allows us to export it in a module, which means that we don't need
to include headers that might expose bugs in Clang's implementation of
C++20 modules.
This takes the function out of the global namespace and keeps the
function names consistent with others in the project.
Rust's [panic macro] is overloaded to be able to take a message. Now
that we've changed the Subspace `sus_panic` macro to a function, we
can create an overload set for `panic`.

[panic macro]: https://doc.rust-lang.org/stable/std/macro.panic.html
This allows us to export it in a module, which means that we don't need
to include headers that might expose bugs in Clang's implementation of
C++20 modules.
This takes the function out of the global namespace and keeps the
function names consistent with others in the project.
This makes it possible for any call to `sus::panic` to be equivalent to
calling the panic handler directly. Users can then have a nice message
emitted during debug builds, and an optimised binary in release builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant