I think this blog post gives a very good overview of the issues around unwrap. I still think it should be in std. The reason here is a structural one: if it weren’t available, people would write similar ad-hoc macros or functions for it.
Defining it as a std lib method enables quite a number of fine grained lints around it.
I still think it should be in std. The reason here is a structural one: if it weren’t available, people would write similar ad-hoc macros or functions for it.
This is what I think as well. I agree with the blog post author that expect should be used instead of unwrap, but unwrap is useful for prototyping, and without it in the standard library, some people would just do .expect("") or as you said define their own function/macro.
I think that the unwrap_used rule should be enabled by default though and that there should be more documentation and guidelines strongly advising against unwrap.
I’m a bit of a Rust noob, but what do you do for a function call that should never fail? For example, I have a static site generator that parses a base URL out of a configuration file (validating that it’s correct). Later, that URL is joined with a constant string: base_url.join("index.html").unwrap(). What do I do here other than unwrap()? Should I use .expect(format!("{}/index.html", base_url)) or similar? It doesn’t make sense to return this as an error via ? because this is an exceptional case (a programmer error) if it happens.
Yes. For a quite a while it would be the line number of the panic call inside unwrap though, so not super useful unless you were running with the RUST_BACKTRACE env var set. This was remedied in Rust 1.42.0.
One easy fix would be to augment cargo publish so that it scans for unwrap. Either disallow the upload or add a tag to the published package, with a link to how to fix this.
There‘s quite a few legitimate uses of unwrap, particularly in cases where the error case is impossible, but is needed e.g. because of a trait interface. Such a solution would be very heavy-handed and I think the practical issues with unwrap are overrated - I rarely see them popping up in projects.
If an error is impossible, you should use std::convert::Infallible (which will be eventually be deprecated in favour the unstable ! (never)) as the error type. Then you could do
fn safe_unwrap<T>(result: Result<T, Infallible>) -> T {
result.unwrap_or_else(|e| match e {})
}
Or with #[feature = "unwrap_infallible"] you can use .into_ok():
fn safe_unwrap<T>(result: Result<T, !>) -> T {
result.into_ok()
}
The “it would be too easy to misuse” line is pretty exhausting to hear for something that isn’t actually unsafe. Result propagation is fantastic and I use it all the time, but it isn’t always appropriate, such as for invariant violation.
Sometimes I just want to unwrap something I believe strongly should not fail; e.g., truncating a usize with what I believe must have a small value into something like a u32. If there was no unwrap() I would have to write expect("superfluous message") which is unlikely to help me, or anybody else, more than the core file or stack trace will. It’s not going to provide the user anything actionable.
Not a rustacean, but it sounds like these uses of unwrap results in undocumented assumptions.
One could address this using comments, but those are subject to decay by various means.
A good “why-oriented” design could have unwrap act like a debug-only expect, making it require a string literal that is ignored when compiling for Release. To facilitate productivity, it could also be made to accept being called without arguments when compiling in Debug mode, falling back to the current unwrap behaviour.
it sounds like these uses of unwrap results in undocumented assumptions
Sort of? It’s true that there isn’t prose as an argument to the call, but sometimes that’s actually fine. The fact that you’ve unwrapped it instead of propagating it as a Result means you’re literally asserting it is invariant for correct operation. You don’t always need more than that; e.g., for converting from a usize to a u32:
let v: u32 = some_usize.try_into().unwrap();
You’re making it clear you believe the usize value will fit inside a u32. Maybe that needs a comment, but sometimes it’s just obvious from the structure of the program why that’s the case (e.g., you assembled a list of five things above, and the usize came from len() of a Vec).
You can write unintelligible or error-prone code in any language. My point is mostly just that, as long as it isn’t demonstrably unsafe, the “if we give it to people they might use it!” justification for not having nice things is pretty weak.
Not a rustacean, but it sounds like these uses of unwrap results in undocumented assumptions. One could address this using comments, but those are subject to decay by various means.
Not really. At the end of the day, if someone writes a helpful function, they don’t know how you’re going to call it, so they have to write it in a general way. But, as the caller, you have more information than the author does, so it’s entirely possible that you’re just calling the function in such a way that you know it can’t fail.
As a not-real example, imagine an integer division function that returns a Result because dividing by zero is an error. If you call that function with a non-zero literal value, you’re not introducing an “undocumented assumption” beyond “I’m assuming that 4 is not 0”.
There’s absolutely no problem with unwrap() in Rust code. I think that people new-and-intermediate to Rust just get over enthusiastic about not having to deal with unchecked exceptions being used for all errors- domain-related and otherwise.
I think this post maybe misses a reason for doing panic.
I tend to prefer crashing over recovering. However expect is mostly more useful than unwrap to provide context. In my production code my “standard” is:
I think this blog post gives a very good overview of the issues around unwrap. I still think it should be in std. The reason here is a structural one: if it weren’t available, people would write similar ad-hoc macros or functions for it.
Defining it as a std lib method enables quite a number of fine grained lints around it.
The most basic one is the one banning it altogether: https://rust-lang.github.io/rust-clippy/master/#unwrap_used
(the search box gives more)
This is what I think as well. I agree with the blog post author that
expectshould be used instead ofunwrap, butunwrapis useful for prototyping, and without it in the standard library, some people would just do.expect("")or as you said define their own function/macro.I think that the
unwrap_usedrule should be enabled by default though and that there should be more documentation and guidelines strongly advising againstunwrap.I’m a bit of a Rust noob, but what do you do for a function call that should never fail? For example, I have a static site generator that parses a base URL out of a configuration file (validating that it’s correct). Later, that URL is joined with a constant string:
base_url.join("index.html").unwrap(). What do I do here other thanunwrap()? Should I use.expect(format!("{}/index.html", base_url))or similar? It doesn’t make sense to return this as an error via?because this is an exceptional case (a programmer error) if it happens.You should just use
unwrap().I think of it like an assertion, if my assumption about this code is wrong, the program will crash. That’s perfectly acceptable in many situations.
or
expect(...)which is identical except for making grepping for the issue later easier if it does come upDoesn’t an unwrap()-induced crash always print the line number in addition to the default error message?
Yes. For a quite a while it would be the line number of the panic call inside unwrap though, so not super useful unless you were running with the
RUST_BACKTRACEenv var set. This was remedied in Rust 1.42.0.One easy fix would be to augment
cargo publishso that it scans forunwrap. Either disallow the upload or add a tag to the published package, with a link to how to fix this.There‘s quite a few legitimate uses of unwrap, particularly in cases where the error case is impossible, but is needed e.g. because of a trait interface. Such a solution would be very heavy-handed and I think the practical issues with unwrap are overrated - I rarely see them popping up in projects.
If an error is impossible, you should use
std::convert::Infallible(which will be eventually be deprecated in favour the unstable!(never)) as the error type. Then you could doOr with
#[feature = "unwrap_infallible"]you can use.into_ok():The “it would be too easy to misuse” line is pretty exhausting to hear for something that isn’t actually unsafe.
Resultpropagation is fantastic and I use it all the time, but it isn’t always appropriate, such as for invariant violation.Sometimes I just want to unwrap something I believe strongly should not fail; e.g., truncating a
usizewith what I believe must have a small value into something like au32. If there was nounwrap()I would have to writeexpect("superfluous message")which is unlikely to help me, or anybody else, more than the core file or stack trace will. It’s not going to provide the user anything actionable.Not a rustacean, but it sounds like these uses of unwrap results in undocumented assumptions. One could address this using comments, but those are subject to decay by various means.
A good “why-oriented” design could have
unwrapact like a debug-onlyexpect, making it require a string literal that is ignored when compiling for Release. To facilitate productivity, it could also be made to accept being called without arguments when compiling in Debug mode, falling back to the current unwrap behaviour.Sort of? It’s true that there isn’t prose as an argument to the call, but sometimes that’s actually fine. The fact that you’ve unwrapped it instead of propagating it as a Result means you’re literally asserting it is invariant for correct operation. You don’t always need more than that; e.g., for converting from a usize to a u32:
You’re making it clear you believe the usize value will fit inside a u32. Maybe that needs a comment, but sometimes it’s just obvious from the structure of the program why that’s the case (e.g., you assembled a list of five things above, and the usize came from
len()of aVec).You can write unintelligible or error-prone code in any language. My point is mostly just that, as long as it isn’t demonstrably unsafe, the “if we give it to people they might use it!” justification for not having nice things is pretty weak.
Not really. At the end of the day, if someone writes a helpful function, they don’t know how you’re going to call it, so they have to write it in a general way. But, as the caller, you have more information than the author does, so it’s entirely possible that you’re just calling the function in such a way that you know it can’t fail.
As a not-real example, imagine an integer division function that returns a Result because dividing by zero is an error. If you call that function with a non-zero literal value, you’re not introducing an “undocumented assumption” beyond “I’m assuming that 4 is not 0”.
There’s absolutely no problem with unwrap() in Rust code. I think that people new-and-intermediate to Rust just get over enthusiastic about not having to deal with unchecked exceptions being used for all errors- domain-related and otherwise.
I agree, see also my above comment for another real-world example.
I think this post maybe misses a reason for doing
panic.I tend to prefer crashing over recovering. However
expectis mostly more useful thanunwrapto provide context. In my production code my “standard” is:panic = 'abort'assert