I found a lint expect_used and it got me thinking. Who even uses this lint? And if someone does, why?
My understanding of error handling strictly divides errors into two classes: bugs and runtime errors (exceptions). So, if I wrote a code like:
let value = collection
.get("key")
.expect("I'm sure it has a `key`, otherwise this is a bug");
I use expect (or unwrap but I prefer expect instead because of clarity). I want it to cause a panic if something wrong happens. Therefore, my code usually looks like a mixture of expect and a Result forwarding:
let value = collection.get("key").expect("has a key");
let file = File::open("foo")?;
But the lint says:
Usually it is better to handle the None or Err case. Still, for a lot of quick-and-dirty code, expect is a good choice, which is why this lint is Allow by default.
It means my strict error handling approach is actually "quick-and-dirty" code
I understand that this lint is allowed by default, but its existence makes me question. Maybe I don't handle errors strictly enough? But most of all, I tend to think that this lint is just useless, or at least has a confusing description.
I am sure that forwarding absolutely all errors up to main (or to a public api of a library) is the wrong way. But if suddenly someone decides to follow the strictest approach as suggested in this video then will it be a good code?
It's not useless – when turned on, it (mostly) prevents you from writing code that panics.
Why are you so confident in this? Especially in a library, it's basically not acceptable to just panic left and right, because then you made an irreversible decision for the caller instead of letting them decide what to do with any eventual errors. Usually, propagating errors as far as possible is in fact the right approach. There may be some exceptions (e.g. io::ErrorKind::Interrupted can just be handled in-place by retrying instead of bubbling it up), but your general attitude towards error handling should be "let them decide".
It depends on the context. Your variant might be right (for specific application). Perhaps the presence of "key" is your collection is some invariant and lack of it is a bug - then expect() is (khm, khm) the expected way of handling it, perhaps even with a strategically placed self-documenting assert!().
Question for people wiser than me: is using assert! family outside of test context a good practice at all? Does it convey any hints for the compiler?
Hm.. In the case of the collection, I could do collection["key"]. But often expect is a more convenient way to do assert when I still need to somehow get a result from a function.
Whether it's good practice shouldn't depend on what the compiler does with it. First and foremost, what matters is semantics. And indeed, since assert!() is guaranteed to fire (not only in debug builds), it can be and is used for e.g. ensuring invariants relied upon by unsafe code. Since the standard library does this all the time, I would say it's perfectly fine to do this in user code, too.
I'm not saying that assert!() doesn't have an effect on optimization, because it does. I'm only saying that you shouldn't decide whether you should use assert!() based on how it optimizes.
Using assertions is standard practice for enforcing all kind of invariants. This obviously includes tests, but also unsafe code, and any code, really. Whenever something should be always true, put an assertion. It will help you a lot while doing any kind of debugging or fuzz testing.
The performance impact of assertions is hard to quantify in the general case, but usually it is negligible. If the condition you're asserting is easy to compute, then it's likely to not affect performance. It may even increase it, since new invariants will allow more optimization opportunities for the compiler.
For example, it is a common trick to encourage autovectorization via strategically placed assertions on the lengths of slices. This works because instead of making some length checks on each element access the compiler can do it only once in the assertion, so element accesses become side-effect free and can be batched via SIMD instrucitons.
Passing errors up to main with ? is only one method of dealing with errors, and it's typically only appropriate for batch jobs or CLI programs where a user is in front of the terminal and able to deal with the error manually. There are lots of other ways of doing things.
For example, in a web server you will often handle errors by returning an appropriate status code (404 for missing resources, 500 for internal errors, etc.). In a kernel or other "mission critical" application, you might handle errors by inspecting the error value and retrying the operation or using a fallback method, or even disabling the offending subsystem altogether so it doesn't bring down the whole system.
I think the main consumers of the expect_used and unwrap_used lints would be that latter group of mission critical applications where all errors must be handled somehow, and panicking is unacceptable. For such applications, being able to forbid unwrap() or expect() would go a long way towards improving reliability.
Using unwrap() or expect() throughout your program means it will panic (crash) the moment it is
in a situation it doesn't expect, and "crashes at the first hint of an edge case" isn't a property you usually associate with robust, well designed programs
I would say expect() isn't necessarily "handling" an error. Rather, it's giving the developer immediate feedback that something happened which they assumed was impossible, and the panic message hints as to what assumption was broken and where so they can fix their code.
The other thing to be aware of is that Rust can be configured to abort on panic, which could cause problems (not sure how severe they could be, honestly) if you're assuming some form of cleanup will happen. Are drop functions called if a program panic-exits "normally" (i.e. panic bubbled up to main)? And not if a panic just aborts the process? Depending on what happens when dropping, that could also cause problems.
I've definitely seen load-bearing assert! to help optimization though, like an early bounds-check to help LLVM see that all the remaining bounds-checks in a loop are redundant and removable.
I think this rather value-laden advice is a bit misleading, especially now Rust has catch_unwind. For some rare situations, it is important that code does not panic. One situation is where a function is called from another language, if you are writing a library and want the functions to be callable from C, you may want to avoid panics. Depending on catch_unwind also precludes the use of the panic = "abort" in your toml file.
That said, I have never encountered such a situation myself, and I use panic, assert, unwrap, indexing and catch_unwind quite freely, and NOT for "quick and dirty" code, but as a perfectly good error-handling method for my circumstances ( which do not include the above cases ). This has not caused me any problems whatsoever, and it can simplify your code somewhat.
Good!
A concrete example: I have an Option value in some context and my invariant is "the value is Some". How can I get a value from Option without unwrap/expect?
I could do this:
if let Some(value) = opt {
// use the value
}
but I'm a big hater of unnecessary nesting. I'd rather do it like this:
let value = match opt {
Some(value) => value,
None => panic!("I need to `handle` None to get the value"),
}
// use the value
This way is better because:
No additional nesting.
If I do make a mistake and break the invariant, I'll find out about it instead of silently doing nothing.
And this is just shorter and does the same thing:
let value = opt.expect("I expect the invariant to be met");
// use the value
clippy::restriction is a lint category for very opinionated lints. Some of which you should never turn on, some of which are outrightcontradictory. unwrap_used/expect_used are restriction lints for a reason: because you're not expected to leave them on. They're to be used to find cases and consider if they're appropriate.
opt.expect("handle should be present") is absolutely fine, if it's a programmer error that opt == None. When you would warn clippy::expect_used is if you are trying to improve type representation of invariants and parse, don't validate; if you know opt.is_some() at some point, in a perfect world, it wouldn't be typed at Option<T>, it'd be typed at T. In a perfect world where you're linting expect_used, having Option<T> means that it could be None, and that you're expected to handle the None case, because if it wasn't/you weren't, you would've just had T.
Sometime in the future, #![feature(let_else)] will make it more reasonable to do the "guard clause" and write
let Some(value) = opt.take()
else {
unreachable!("`opt` should only be taken from by one path");
}
and this also gives you a block to error! or otherwise provide more formatting context than .expect's single &str, or perhaps to change to yeet SomeError::new() if it turns out that you were wrong.
