Stream: ideas

Topic: Say no to panic!

Locria Cyber (Nov 01 2021 at 06:06):

Please stop using panic! all over the place! Here's a better choice: https://docs.rs/anyhow/1.0.44/anyhow/

Matthias Beyer (Nov 01 2021 at 06:28):

I agree! Latest trunk has 453 panic!() calls! That's just insane! :laughing: (to be fair: I checked with grep so there might be false positives in this number)

Locria Cyber (Nov 01 2021 at 07:03):

Here's a tool that stops compilation after first error

Locria Cyber (Nov 01 2021 at 07:03):

cargo watch -cs "cargo first run"

Locria Cyber (Nov 01 2021 at 07:04):

cargo install cargo-watch cargo-first

Locria Cyber (Nov 01 2021 at 07:04):

I'm working on making roc_cli panic-free

Locria Cyber (Nov 01 2021 at 07:53):

https://github.com/rtfeldman/roc/pull/1859

Matthias Beyer (Nov 01 2021 at 08:43):

I am now trying to make /compiler/build panic-free!

Folkert de Vries (Nov 01 2021 at 09:38):

We already have error formatting in the reporting crate. We'd want to use a consistent style across all error messages

Folkert de Vries (Nov 01 2021 at 09:48):

also for development we absolutely need the source location where the panic occured. Does anyhow provide that?

Folkert de Vries (Nov 01 2021 at 09:49):

I mean the rust source location, panics give file and line number

Richard Feldman (Nov 01 2021 at 12:04):

yeah the #1 reason we've used panic! and todo! is as a placeholder for "doing this properly will take significantly longer, so let's get something working and revisit this later"

Richard Feldman (Nov 01 2021 at 12:05):

so if anyone wants to revisit them and make them do the right thing instead, that would be awesome! :heart_eyes:

Richard Feldman (Nov 01 2021 at 12:05):

however, the right thing is not "replace panic with anyhow and call it a day" :big_smile:

Richard Feldman (Nov 01 2021 at 12:06):

it's going to vary a lot on a per-panic basis!

Richard Feldman (Nov 01 2021 at 12:07):

e.g. I know in several cases it's "we already have a nice error handling system in place, but we didn't take the time to assemble the necessary data structures it needs, and just panicked instead for now"

Richard Feldman (Nov 01 2021 at 12:09):

and in others it's "we handle 80% of the branches of this pattern match but not all of them, so the actual solution is not to error out at all, but rather to finish implementing the other branches"

Richard Feldman (Nov 01 2021 at 12:09):

etc

Richard Feldman (Nov 01 2021 at 12:13):

so @Matthias Beyer I love the idea of making more things panic-free, and reviewing a PR for that would make me super happy! 🤩

I would recommend following a guideline of "don't introduce any new error handling dependencies" (such as anyhow) without discussing first, because otherwise I there's a high risk of the PR feedback being "this isn't how we want to handle these errors; actually it's ______" which would be easy to avoid with a little discussion instead! :smiley:

Locria Cyber (Nov 01 2021 at 13:26):

Folkert de Vries said:

also for development we absolutely need the source location where the panic occured. Does anyhow provide that?

Yes

Locria Cyber (Nov 01 2021 at 13:26):

Most of the panic is caused by unwrap.

Locria Cyber (Nov 01 2021 at 13:28):

With anyhow, every error can be caused by other errors. I'll try not to alter existing error reporting behavior.

Richard Feldman (Nov 01 2021 at 13:28):

ah damn I just saw your PR @Locria Cyber - sorry, I wish we'd talked about this sooner!

Locria Cyber (Nov 01 2021 at 13:29):

We can still talk about this now.

Richard Feldman (Nov 01 2021 at 13:29):

Yeah let's!

Richard Feldman (Nov 01 2021 at 13:30):

I have some time but will have to go in 10ish minutes (but happy to talk more about it tonight)

Locria Cyber (Nov 01 2021 at 13:30):

I added MaybeError that describe either a typed error that will be handled or any error.

Richard Feldman (Nov 01 2021 at 13:32):

so fundamentally my concern with anyhow and similar solutions is that we're turning our placeholders into concrete long term versions of the placeholder

Locria Cyber (Nov 01 2021 at 13:32):

For example, there is this
https://github.com/rtfeldman/roc/blob/9d53e1d748ab6ba3b057158415db6ffeb64fd721/cli/src/repl/gen.rs#L210

Locria Cyber (Nov 01 2021 at 13:33):

gen_and_eval should probably be two functions though

Richard Feldman (Nov 01 2021 at 13:33):

this change is an example of what I'm talking about

Locria Cyber (Nov 01 2021 at 13:34):

https://github.com/rtfeldman/roc/blob/0d381c6bf444758431cf76666cb3377538f54b72/cli/src/repl/gen.rs#L215

Richard Feldman (Nov 01 2021 at 13:34):

this is changing Result<BuiltFile, LoadingProblem<'a>> into Result<BuiltFile, MaybeError<LoadingProblem<'a>>>

Richard Feldman (Nov 01 2021 at 13:34):

but we already have a concept of a loading problem - that's what LoadingProblem does! :big_smile:

Richard Feldman (Nov 01 2021 at 13:34):

so this change wraps our existing error handling mechanism in a new one, and what the new one does is to take a TODO error message and present it to the user

Richard Feldman (Nov 01 2021 at 13:35):

what I think we should actually do here is incorporate this error into LoadingProblem, and present it to the user in a nice way

Richard Feldman (Nov 01 2021 at 13:35):

so this is what I mean when I say I think introducing anyhow is fundamentally not how we want to be handling these

Richard Feldman (Nov 01 2021 at 13:36):

in most cases the problem isn't "panic is worse than Result" but rather "we aren't handling the error gracefully, and we should handle it gracefully instead"

Richard Feldman (Nov 01 2021 at 13:36):

and the graceful error handling mechanism is something other than anyhow :big_smile:

Folkert de Vries (Nov 01 2021 at 13:37):

to add, some errors are meant to be panics: they indicate internal issues, and for debugging purposes we want them to be panics, or at least present the same information. I don't see how anyhow could get hold of a file/line number in this context

Richard Feldman (Nov 01 2021 at 13:37):

yeah, actually I think we do want an ice! macro (short for Internal Compiler Error) - rustc uses this a bunch

Locria Cyber (Nov 01 2021 at 13:37):

Folkert de Vries said:

to add, some errors are meant to be panics: they indicate internal issues, and for debugging purposes we want them to be panics, or at least present the same information. I don't see how anyhow could get hold of a file/line number in this context

std::error::Error has traceback builtin.

Folkert de Vries (Nov 01 2021 at 13:38):

yea that sounds good. I believe the elm compiler also has some panic catcher

Richard Feldman (Nov 01 2021 at 13:38):

it's a panic that prints a message like "This is a compiler bug - it should not have happened, so please report this error! Here's how to do that"

Locria Cyber (Nov 01 2021 at 13:38):

Richard Feldman said:

it's a panic that prints a message like "This is a compiler bug - it should not have happened, so please report this error! Here's how to do that"

I already tried https://crates.io/crates/human-panic and it doesn't work

Richard Feldman (Nov 01 2021 at 13:39):

oh I don't think we need a crate for that!

Folkert de Vries (Nov 01 2021 at 13:39):

final consideration: dependencies slow down our builds.

Richard Feldman (Nov 01 2021 at 13:39):

we just define our own macro named ice! and all it does is to call panic! with some stock message around it like "Hey, you hit a compiler bug! We didn't think it was possible that this would happen, sorry about this...here's how to report it"

Locria Cyber (Nov 01 2021 at 13:40):

not really

Richard Feldman (Nov 01 2021 at 13:40):

@Locria Cyber why do you say that?

Locria Cyber (Nov 01 2021 at 13:40):

The bevy game engine compiles very fast by designing code for fast compilation

Locria Cyber (Nov 01 2021 at 13:41):

currently most of codegen happens in roc_cli(bin). That's probably a sign for macro/link time

Richard Feldman (Nov 01 2021 at 13:41):

be that as it may, it's a fact that adding dependencies makes the build slower :big_smile:

Folkert de Vries (Nov 01 2021 at 13:41):

(it's linking time mostly, yes)

Locria Cyber (Nov 01 2021 at 13:42):

It's acceptable since it's boilerplate you have to write anyways?

Richard Feldman (Nov 01 2021 at 13:42):

but regardless, I don't think an ice! macro needs anything that's not in std to do what we want

Locria Cyber (Nov 01 2021 at 13:42):

Just calling panic won't show the reason up to causing the error

Locria Cyber (Nov 01 2021 at 13:43):

You only get the stack trace

Folkert de Vries (Nov 01 2021 at 13:43):

what else would you want/need?

Locria Cyber (Nov 01 2021 at 13:44):

With anyhow you get like

Error: Cannot open file <line number>
Context: when trying to load module <line number>
Context: when processing repl <line number>
Context: ...

Richard Feldman (Nov 01 2021 at 13:45):

(I gotta go, as previously mentioned, but I'll catch up on this discussion tonight! :wave:)

Folkert de Vries (Nov 01 2021 at 13:45):

hmm, so I just get that information from the stack trace as I investigate

Folkert de Vries (Nov 01 2021 at 13:46):

but I'm very familiar with the codebase so maybe I'm biased

Locria Cyber (Nov 01 2021 at 13:46):

And you can intercept all this information and

1. let it bubble up to main and show default panic (above message)
2. print Roc-theme error message for every possible message based on dynamic dispatch
3. print specific error message like a variable is declared twice here and here

Locria Cyber (Nov 01 2021 at 13:47):

also how to use another linker under nix

Locria Cyber (Nov 01 2021 at 13:47):

mold is fast

Folkert de Vries (Nov 01 2021 at 13:48):

mold should be fast, but I did not get big speedups when I tried it a while ago. Maybe something changed though

Locria Cyber (Nov 01 2021 at 13:49):

mold is really fast

Lucas Rosa (Nov 01 2021 at 14:17):

I hope this doesn't come off the wrong way but I strongly recommend getting more intimate with the project first before making sweeping changes on the basis of "obviously correct", many things have quite a lot of context behind the decisions. But really this is easily resolved by bringing things up in a conversation before spending time on code that may not end up getting merged. There's many builtins still to be implemented and that's generally a great way to learn how things work in the compiler, if you want some help adding a builtin feel free to shoot me a DM and we can set up a time. If you prefer to work solo, then any previous PR for a builtin should act as a good template. This also isn't to say your suggestion isn't a good idea, but it's sad to have to close work that someone spent time on due to a lack of communication before hand.

Brendan Hansknecht (Nov 01 2021 at 20:14):

So I used to work professionally with Go where a lot of emphasis is placed on errors, error handling, and error wrapping/propagating. How our errors worked actually closely represented the examples @Locria Cyber mentioned with anyhow.
I would say that, for the most part, changing to something like that would not benefit Roc. Roc has what I would label as 3 main classes of errors Todo/unimplemented, internal compiler errors, and user errors.
Todo should all eventually be removed and don't matter for long term error tracking.
User errors should be cleanly displayed to the user and often times, in code, might not even be represented as an error. These errors often need to be custom with extra tracking information in order to make a pretty user error message. They should never need stack trace information or compiler internals. If we have to show a stack trace to a user, there is a bigger problem with our design. A number of user errors in roc are currently not represented correctly because it is more work to properly report an error than just panic and come back to it later. It would probably be a good idea to distinguish needing better reporting and actual compiler internal errors.
Compiler internal errors should never be hit, but exist because people write bugs and make wrong assumptions. This is the case where an panic, unwrap, or expect happens to be wrong but never should be. If the error is expected to be possible, this should be upgraded to a user error. A lot of cases where we use expect and explain what happened most likely should be switched to user errors, but it depends on context.
Compiler internal errors are where something like anyhow might be useful. I think it would be most useful to new compiler developers. The main advantage it gives is an annotated stack trace instead of a raw stack trace. In my experience, naming in wrapped errors tends to be not much more useful than a stack trace, but for new users, it can jumpstart their understanding as opposed to feeling lost in a sea of functions that have a good chance to not even have documentation.

Richard Feldman (Nov 01 2021 at 21:00):

There's many builtins still to be implemented and that's generally a great way to learn how things work in the compiler, if you want some help adding a builtin feel free to shoot me a DM and we can set up a time.

this is a great suggestion and I highly recommend pairing with Lucas in general! :smiley:

Richard Feldman (Nov 01 2021 at 21:02):

Roc has what I would label as 3 main classes of errors Todo/unimplemented, internal compiler errors, and user errors.

I totally agree with this, and with the description of how we should handle the 3 categories of errors - love it! :100:

Richard Feldman (Nov 01 2021 at 21:05):

Compiler internal errors are where something like anyhow might be useful. I think it would be most useful to new compiler developers. The main advantage it gives is an annotated stack trace instead of a raw stack trace. In my experience, naming in wrapped errors tends to be not much more useful than a stack trace, but for new users, it can jumpstart their understanding as opposed to feeling lost in a sea of functions that have a good chance to not even have documentation.

yeah, although this is where I'm a bit torn, because that benefit is not free; there's some amount of runtime overhead compared to panic! (a small amount of overhead in isolation, but multiplied by a lot of call sites) and it also makes the types more involved in that it can make a function that should theoretically "always succeed" (unless there's a bug somewhere) return a Result

Richard Feldman (Nov 01 2021 at 21:05):

also it's one more dependency, which slows down builds incrementally

Richard Feldman (Nov 01 2021 at 21:07):

so while I definitely acknowledge that there's a benefit, I think it's pretty small (in the very specific case of an internal compiler bug, it gives more diagnostic context to compiler authors than a stack trace alone), and I'm not convinced that benefit outweighs the costs

Brendan Hansknecht (Nov 01 2021 at 21:13):

I agree. Just noting where I see a valid use case.
As a side note, we might at some point want to do some sort of larger compilation speed optimization project. Like actually do deeper analysis and try to get larger changes done that could speed up compilation time. Things like: removing/swapping crates, minimizing used features, using 1 version of a crate everywhere, reorganizing our crates for better dependency chains, looking at other tips and tricks. Generally speaking compilation time slowly rises, and it can be hard to make a dent without a larger group effort.

Lucas Rosa (Nov 01 2021 at 23:46):

FWIW, I've used anyhow before and it was nice. but I understand being hesitant to add new dependencies. @Brendan Hansknecht That error category break down would make for a pretty nice blog post :P

Locria Cyber (Nov 03 2021 at 06:16):

I really hope there is an easy way to track all the functions that just panic!

Locria Cyber (Nov 03 2021 at 06:17):

no_panic is quite useless since it only tells you one function that might panic! while not telling you where inside the function.

Locria Cyber (Nov 03 2021 at 06:28):

I think right now I want to

1. make a reproducible build environment that uses mold (if the build is fast enough, then we can add dependencies)
2. Define error categories with @Lucas Rosa and how they should be handled in code
3. use an existing tool (or make one) for checking errors

Locria Cyber (Nov 03 2021 at 07:33):

Actually, I'll be unavailable over a month.

Brendan Hansknecht (Nov 03 2021 at 16:06):

if the build is fast enough, then we can add dependencies

Just a general note, we are dependency adverse, but we do add new dependencies. Generally, it is a measure of trade-offs and value. I wouldn't label this as a build time issue. Yes we want faster build times, but we won't restrict ourselves from ever adding dependencies due to build times rising.

Brendan Hansknecht (Nov 03 2021 at 16:10):

Also as a starting point here, it might be a good idea to make 2 macros that wrap panic!:

• One for internal compiler errors that adds a url and asks the user to file a bug. It would be great if we could make also make a modified version of unwrap/expect that call this (if that is even possible).
• The other version would specifically mark that a panic! should be replaced by a proper user error message. This way, those errors will be track directly and we can slowly work to replace all of these cases. It will make it much easier for new users to know locations where it would be a great contribution to switch to roc_reporting or similar.

This would enable us to encode some of the domain specific knowledge around when we do or don't want a panic!. Overall, it should make it significantly easier to make future changes without changing the wrong type of panic!.

Richard Feldman (Nov 03 2021 at 16:23):

yeah I like ice! for the former, like rustc uses, and maybe we could do roc_todo! for the latter?

If we wanted to get really fancy, we could have roc_todo! take a u64 as its first argument, which would be an issue number - so the expectation would be that you'd open an issue for each new usage of the macro, which would make it easier for others to discover later when looking through the issue tracker

Richard Feldman (Nov 03 2021 at 16:25):

once we'd converted everything over, we could also have a CI step that verifies we're not calling raw panic! or todo! - or even unwrap or expect (so we'd have to explicitly use unwrap_or_else with unreachable! if we're really sure, which personally I'd be ok with)

Brendan Hansknecht (Nov 03 2021 at 16:27):

As long as we are fine with them having the same issue number, that sounds fine. For example, the linker would likely use it for most of it's current panics that just want better error reporting. Probably would just want one issue for that.
Also, not sure I would call it "todo". Since I think it should be explicitly for things we want to generate nice user errors for and not actual todos.

Locria Cyber (Nov 08 2021 at 06:15):

Richard Feldman said:

once we'd converted everything over, we could also have a CI step that verifies we're not calling raw panic! or todo! - or even unwrap or expect (so we'd have to explicitly use unwrap_or_else with unreachable! if we're really sure, which personally I'd be ok with)

The current state-of-the-art is no_panic crate, which will error out at link time. So you have to compile the whole project before knowing that your program will panic without further information.

We would need to use static analysis for faster feedback, which calls for a new tool, probably with help from rust-analyzer.

Last updated: Jun 16 2026 at 16:19 UTC