4 ms·

> `unwrap()` is "handling" errors by crashing the program, so it shouldn't really be used This isn't the right advice to give, and is only going to confuse beg

> `unwrap()` is "handling" errors by crashing the program, so it shouldn't really be used This isn't the right advice to give, and is only going to confuse beginners. Panicking via unwrap/expect is perfectly fine, whether in a library or an application, only when the panic represents a bug. For example, accessing an element of a Vec with an incorrect index because of a logic error. If a panic occurs, then it should reflect a bug that is intended to be fixed. Stated differently, it should be impossible for an end user to cause a Rust application to panic. If they can, then it's a bug. This advice permits use of unwrap/expect anywhere, so long as its occurrence is never expected.

jayflux 7y ago
I’m pretty sure clippy “encourages” you to use .get() for Vectors returning an Option<> for you to deal with so accessing a bad index is avoidable in idiomatic rust. I’m not sure that was the best example but I agree with your overall point.
- GrayShade 7y ago
  It doesn't: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=850feb616235ea3cfc2f841629937eee https://play.rust-lang.org/?version=stable&mode=debug&editio... (click Tools / Clippy).
- burntsushi 7y ago
  It does not. I know because I advocated against it. :-) https://github.com/rust-lang/rust-clippy/issues/1300 https://github.com/rust-lang/rust-clippy/issues/1300 Also, Clippy's defaults are developed to be quite opinionated. It makes many suggestions that I (and many others) disagree with. So Clippy's defaults should not be used as a bludgeon for what's idiomatic and correct.
  - steveklabnik 7y ago
    (I also do not use clippy very aggressively due to disagreeing with a lot of lints)
bluejekyll 7y ago
I mostly agree with you, but in practice I think using unwrap or panicking in libraries is usually wrong. As an example it’s quite easy to fall astray of taking your perspective to say read a packet off the network, and panic on malformed data. I know you’d agree that would be a inappropriate time to use panic, as that would crash any upstream program with trivial DOS exploits. But this is easy to do if you say have an API that takes a value, translates it to something else and panics on invalid inputs. My advice is generally in line with the GP, pornel—use the tools at hand for avoiding panics where it’s easy to do so, such as ?, map, etc. Only panic/unwrap on bad library API usage (which I think is what you’re suggesting). Feel free to panic anywhere in main. People often translate “ use of unwrap/expect anywhere, so long as its occurrence is never expected” in ways that can create major bugs as the software goes into more use.
- burntsushi 7y ago
  I understand that perspective to an extent, but I really want to double down here because I think it's important. :-) > but in practice I think using unwrap or panicking in libraries is usually wrong Very strongly disagree. I've created dozens of Rust libraries, and probably all of them have dozens of code paths that can panic. Other core libraries do the same. Just a simple slice access, e.g., `slice[i]`, is a line of code that could panic. (Since it's just a shorter way of writing `slice.get(i).unwrap()`.) I don't think we should be giving advice that runs contrary to how important libraries actually work. A lot of people learn to code by reading others' code, and when we give advice like "don't use unwrap/expect," they get confused when they see that virtually every widely used piece of Rust code violates it. The key here is really that one shouldn't panic unless the panic itself is indicative of a bug somewhere. Blanket advice like "don't use unwrap/panic in libraries" is bad because---as I argued in the Clippy issue I posted in a sibling comment---it's effectively a prohibition against runtime invariants themselves. As a programmer, you insert a panic when you've failed (for any number of reasons) to capture the invariant in the type system. Blanket advice saying that one shouldn't use unwrap/expect in these circumstances leads one toward a path of much more complex APIs with error types that are never constructed unless there is a bug in the code. (It's likely that actually adding all of those error types will be so annoying that it's plausible the programmer will give up on Rust.) > As an example it’s quite easy to fall astray of taking your perspective to say read a packet off the network, and panic on malformed data. I know you’d agree that would be a inappropriate time to use panic, as that would crash any upstream program with trivial DOS exploits. But this is easy to do if you say have an API that takes a value, translates it to something else and panics on invalid inputs. I think this is a reflection of error handling being hard. It's just as easy to code a program with the mistaken assumption that a particular file path will always point to a valid and readable file. It takes a bit of learning to understand which things you can rely on never happening and which you can't. In the mean time, I don't think we should be giving advice that is very easy to misinterpret into a conclusion that just isn't tenable. Personally, I think it's much easier to talk about end user behavior. If there's a panic, then it's a bug that ought to be fixed. If you follow that, it's hard to go wrong. You could rephrase it with, "If there's a reachable code path that panics, then the code path should be removed." You could then talk about what "reachable" means, i.e., code paths that are determine based on data that the program doesn't control vs code paths that are enforced for all inputs to the program.

jayflux 7y ago

I’m pretty sure clippy “encourages” you to use .get() for Vectors returning an Option<> for you to deal with so accessing a bad index is avoidable in idiomatic rust. I’m not sure that was the best example but I agree with your overall point.

GrayShade 7y ago
It doesn't: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=850feb616235ea3cfc2f841629937eee https://play.rust-lang.org/?version=stable&mode=debug&editio... (click Tools / Clippy).
burntsushi 7y ago
It does not. I know because I advocated against it. :-) https://github.com/rust-lang/rust-clippy/issues/1300 https://github.com/rust-lang/rust-clippy/issues/1300 Also, Clippy's defaults are developed to be quite opinionated. It makes many suggestions that I (and many others) disagree with. So Clippy's defaults should not be used as a bludgeon for what's idiomatic and correct.
- steveklabnik 7y ago
  (I also do not use clippy very aggressively due to disagreeing with a lot of lints)

bluejekyll 7y ago

I mostly agree with you, but in practice I think using unwrap or panicking in libraries is usually wrong. As an example it’s quite easy to fall astray of taking your perspective to say read a packet off the network, and panic on malformed data. I know you’d agree that would be a inappropriate time to use panic, as that would crash any upstream program with trivial DOS exploits. But this is easy to do if you say have an API that takes a value, translates it to something else and panics on invalid inputs. My advice is generally in line with the GP, pornel—use the tools at hand for avoiding panics where it’s easy to do so, such as ?, map, etc. Only panic/unwrap on bad library API usage (which I think is what you’re suggesting). Feel free to panic anywhere in main. People often translate “ use of unwrap/expect anywhere, so long as its occurrence is never expected” in ways that can create major bugs as the software goes into more use.

burntsushi 7y ago
I understand that perspective to an extent, but I really want to double down here because I think it's important. :-) > but in practice I think using unwrap or panicking in libraries is usually wrong Very strongly disagree. I've created dozens of Rust libraries, and probably all of them have dozens of code paths that can panic. Other core libraries do the same. Just a simple slice access, e.g., `slice[i]`, is a line of code that could panic. (Since it's just a shorter way of writing `slice.get(i).unwrap()`.) I don't think we should be giving advice that runs contrary to how important libraries actually work. A lot of people learn to code by reading others' code, and when we give advice like "don't use unwrap/expect," they get confused when they see that virtually every widely used piece of Rust code violates it. The key here is really that one shouldn't panic unless the panic itself is indicative of a bug somewhere. Blanket advice like "don't use unwrap/panic in libraries" is bad because---as I argued in the Clippy issue I posted in a sibling comment---it's effectively a prohibition against runtime invariants themselves. As a programmer, you insert a panic when you've failed (for any number of reasons) to capture the invariant in the type system. Blanket advice saying that one shouldn't use unwrap/expect in these circumstances leads one toward a path of much more complex APIs with error types that are never constructed unless there is a bug in the code. (It's likely that actually adding all of those error types will be so annoying that it's plausible the programmer will give up on Rust.) > As an example it’s quite easy to fall astray of taking your perspective to say read a packet off the network, and panic on malformed data. I know you’d agree that would be a inappropriate time to use panic, as that would crash any upstream program with trivial DOS exploits. But this is easy to do if you say have an API that takes a value, translates it to something else and panics on invalid inputs. I think this is a reflection of error handling being hard. It's just as easy to code a program with the mistaken assumption that a particular file path will always point to a valid and readable file. It takes a bit of learning to understand which things you can rely on never happening and which you can't. In the mean time, I don't think we should be giving advice that is very easy to misinterpret into a conclusion that just isn't tenable. Personally, I think it's much easier to talk about end user behavior. If there's a panic, then it's a bug that ought to be fixed. If you follow that, it's hard to go wrong. You could rephrase it with, "If there's a reachable code path that panics, then the code path should be removed." You could then talk about what "reachable" means, i.e., code paths that are determine based on data that the program doesn't control vs code paths that are enforced for all inputs to the program.