On running
cargo clippy -- -W clippy::panic
I get warnings ( whenever the panic! macro is invoked )
warning: panic should not be present in production code
I thought this was a very strange message. Is my understanding all wrong? Isn't it correct to panic when something has gone wrong, even in production code?
There's a reason the lint is turned off by default.
clippy::panic is part of the restriction lint group: Clippy Lints This lint group lists a lot of lints that are only useful for specific projects. It also contains lints restricting any printing to stdout/stderr, restricting the use of the modulo operator (%), any many more.
I understand it's not a lint that would often be used. The issue I have is the wording of the message. I guess it could say "To minimise code size and maximise performance you could consider removing this invocation of panic!", or something to that effect. However to baldly suggest or state that ALL checks (panics) should be disabled in production code seems very controversial, and even against everything that Rust stands for. Unless I have misunderstood something, which is always possible.
Well, you would only enable that lint if you believe that panics should not exist in production code, so the message makes sense for the people who would enable it.
People may utilize panic!() to minimize code size and maximize performance with panic = "abort" to avoid all those Result handlings.
That's not (primarily) why the lint is there. Panicking crashes the program, so it's best if you can write code that is statically known not to panic. It improves reliability, first and foremost. Panicking doesn't really have a performance hit unless it actually happens (at which point it probably doesn't matter anyway); the panicking infrastructure is almost universally marked as #[cold], #[inline(never)], or similar, in order not to cause unnecessary inlining, cache pressure, and binary bloat.
Please avoid insults. They do not lead to productive discussion.
That's not what I meant, and it's not what the lint warns about. The point is that you should handle errors rather than just letting the program crash. This mostly entails using Result-returning APIs instead of their panicking counterparts (e.g. <[T]>::get() instead of indexing, result? instead of result.unwrap(), etc.)
For a large class of errors ( things that should never happen if the program is correct, such as an array index check failing, or a borrow check failing, or any number of internal checks a program may carry out ) the correct way to handle an error is to panic. That's what it's for. When the program has detected that there is a software error (or possibly the hardware has failed!), there is usually no sensible way to continue.
That's quite the assertion. "The" correct way implies that anything else is an "incorrect" way of handling errors. I strongly disagree.
Granted, there are situations where the easy solution is to panic. And by "easy", I mean the solution requiring the least amount of refactoring/re-architecting of the code. However, panicking being practically fatal (unless someone expecting the code to panic specifically wraps it in catch_unwind) is very inconvenient and can lead to a lot of annoyance or even worse consequences.
For example, crashing a program in a hard-realtime system that controls something like an on-board computer for a vehicle could literally be fatal, i.e. people may die. Therefore, panicking in such kind of control software is absolutely unacceptable, and should be replaced with graceful failure: retrying, controlled partial restarting of certain routines, or only stopping a single operation, but continuing as many of them as possible, while reliably reporting the failure to the user.
But even if we don't consider such drastic circumstances, panicking can still be a bad idea. For example, in a web service, where the server needs to be kept up and running, having the whole thing grind to a halt just because some request handler had a bug and panicked is clearly too much.
This is not true, either. In a compiler for a statically-typed database query language I have written, I needed to use many Rc<RefCell>s for representing types. Naturally, using these required borrowing the RefCell all the time. Instead of calling borrow() and borrow_mut(), I always used try_borrow() and try_borrow_mut(), and mapped the error case to a diagnostic message that informed the user of an internal bug in the compiler.
It was totally easy to do, and even though these conditions were expected never to occur, this approach avoided unwanted crashes, allowed the compiler to clean up its internal state, dump it to help debugging, and inform the user with a nicely-formatted, colored error message that fit into the rest of the output, instead of an ugly, raw Debug representation of a non-informative error message.
Well, that's non-local handling of the error, and it's now easy to do with panic::catch_unwind. Perhaps you wrote the compiler before catch_unwind was stable ( and perhaps the panic lint was conceived before catch_unwind was available). It has the advantage of catching all errors.
In the case of vehicle control, I would expect some kind of software restart to be attempted. Watching Formula I races, I sometimes wonder what is going on when a car grinds to a halt, apparently due to some software issue!
No. panic_unwind() is not meant to be a general error handling mechanism. In particular, it is not reliable: in a panic = "abort" profile, it is a no-op. It is also easy to cause UB by globally relying on unwinding in code that interacts with FFI, and, relatedly, it is not possible to use !UnwindSafe types with catch_unwind().
Again, Result is the primary, general, first-line error handling mechanism in Rust, and panicking should not be considered a substitute or an equal alternative for it. All of this is spelled out explicitly in the documentation of catch_unwind().
It states " The Result type is more appropriate to use for functions that can fail on a regular basis."
"Regular basis" is not defined, but a software or hardware error should not occur on a regular basis, if the program is correct, there shouldn't be software errors at all, let alone on a regular basis. So handling such errors by panic rather than Result seems quite appropriate and consistent with the panic_unwind documentation.
This wording is definitely confusing. Pedantically, it is wong, as it's totally OK for production code to contain calls to panic! in some specific cases. Can someone create an issue on the clippy repo to clarify the wording?
This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.