crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.2k stars 1.61k forks source link

Add `Crystal::System.panic` #14733

Closed ysbaddaden closed 5 days ago

ysbaddaden commented 1 week ago

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

straight-shoota commented 1 week ago

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).

ysbaddaden commented 6 days ago

@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.

jkthorne commented 6 days ago

Why is this under the Crystal::System namespace?

Some languages like go have panic way more accessible.

straight-shoota commented 6 days ago

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.

jkthorne commented 4 days ago

That makes sense. Thanks for the explanation.