Skip to content

Adopt ABORT throughout the compiler #81604

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 9 commits into
base: main
Choose a base branch
from

Conversation

hamishknight
Copy link
Contributor

@hamishknight hamishknight commented May 19, 2025

Turn abortWithPrettyStackTraceMessage into the wrapping macro ABORT, then convert a bunch of places where we're dumping to stderr and calling abort (including ASSERT and friends) over to using this new macro such that the message gets printed to the pretty stack trace. This ensures it gets picked up by CrashReporter.

@hamishknight hamishknight requested a review from tbkka May 19, 2025 10:57
@hamishknight hamishknight changed the title Adopt abortWithPrettyStackTraceMessage throughout the compiler Adopt ABORT throughout the compiler May 19, 2025
Improve the formatting by indenting to match the rest of the
pretty stack trace.
It's not clear this is a useful utility to expose since it will only
print the message once.
Turn it into a wrapping macro that includes the file location, and
move into `Assertions.h`.
Make sure we still output the error message when pretty backtracing
is disabled.
Ensure the message gets printed to the pretty stack trace, allowing
it to be picked up by CrashReporter.
We already setup a PrettyStackTrace to include the generic signature
on a crash.
Convert a bunch of places where we're dumping to stderr and calling
`abort` over to using `ABORT` such that the message gets printed to
the pretty stack trace. This ensures it gets picked up by
CrashReporter.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

RequirementMachine changes LGTM

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.

2 participants