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.

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

Add Crystal::System.panic #14733

Merged
merged 5 commits into from
Jun 26, 2024

Conversation

ysbaddaden
Copy link
Contributor

Prints a system error message on the standard error then exits with an error status. Raising an exception should always be preferred but there are a few cases where we can't allocate any memory (e.g. stop the world) and still need to fail when reaching a system error.

Extracted from #14729

Prints a system error message on the standard error then exits with an
error status. Raising an exception should always be preferred but there
are a few cases where we can't allocate any memory (e.g. stop the world)
and still need to fail when reaching a system error.
@straight-shoota
Copy link
Member

I'm wondering if we could simplify the implementation a bit? The code seems to be mostly identical across platforms with the only difference in retrieving the error message allocation-free.
Perhaps we could extract this for Errno and WinError (WasiError.message already does not allocate).
Some APIs on Windows return Errno, so it might make sense to have a fixed API like SystemError, accepting any system error value (Errno | WinError | WasiError).

@straight-shoota straight-shoota changed the title Add Crystal::System.panic Add Crystal::System.panic Jun 24, 2024
@ysbaddaden
Copy link
Contributor Author

@straight-shoota You're right. I thought I couldn't abstract a method without allocating... but all I had to do was yield. The method also takes an explicit Errno|WinError|WasiError param. That simplified the panic method a lot.

Note: a follow-up PR will introduce strerror_r (thread-safe) instead of strerror (thread unsafe) when available.

@straight-shoota straight-shoota added this to the 1.13.0 milestone Jun 24, 2024
@jkthorne
Copy link
Contributor

Why is this under the Crystal::System namespace?

Some languages like go have panic way more accessible.

@straight-shoota
Copy link
Member

straight-shoota commented Jun 25, 2024

It's a low level primitive for stdlib implmentations. It's probably not very practical for user code. Error handling is usually intended to work with exceptions. And .abort is a similar, public method for exiting directly with an error message.

@straight-shoota straight-shoota merged commit 6c34494 into crystal-lang:master Jun 26, 2024
61 checks passed
@ysbaddaden ysbaddaden deleted the feature/system-panic branch June 26, 2024 11:06
@jkthorne
Copy link
Contributor

That makes sense. Thanks for the explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants