Hacker News Reader

Comment by steveklabnik | Hacker News Reader

unwrap has legitimate use-cases, and it's not clear what "disable" it would be, as it changes the type of the thing it returns. You could write a lint to fail the build, if you want, I guess...

View on HN

.unwrap() is only the right choice if you need to optimize for binary size and can't afford the cost of the precise error message you would pass to .expect(). There are situations where you can't possibly continue running the application if an error occurs, but you shouldn't rely on a backtrace (which you might not manage to capture, e.g. if RUST_BACKTRACE is unset or you don't have symbols) as your only method of communication with your future self.

This is pedantic. It seems clear from context that this conversation is about panicking on the None/Err cases, and unwrap is a shorthand for "unwrap or except or match with a panic branch."

This is not true. For example, consider this code:

    if foo.is_some() {
        let foo = foo.unwrap();
    } else {
        // other code
    }

Here, I _know_ that foo is some. The extra error message from expect will _never_ be seen.

Now, this is a contrived example, and would better be written with `if let` in today's Rust, but this is the _kind_ of situation in which unwrap is totally, 100% cool, but the compiler can't know.

I write Swift daily and I just don't force unwrap anymore, ever. I don't think a hard crash is very usable in a production application, a lot of people disagree and want a hard crash while testing but I think for those bugs that slip through the user experience between for example "loading the first screen but my avatar isn't set" is so much better than "loading the first screen and the app kills itself" just because you force unwrapped the URL of the avatar from the JSON response that had a slight problem in production.

Well perhaps we should have something that logs the error in production but keeps on trucking and crashes the application when the debug or test flag is set?

panics are explicitly for unrecoverable errors, so recovering from them and keeping on going means that you're not using the right kind of error handling. If that's the behavior you'd want, then you wouldn't want to use unwrap.

The error message in this case might be something like "foo became None after verifying it to be Some". This could happen, for example, if incorrect unsafe code in another thread concurrently mutates foo through a raw pointer. My point is that of course while writing them you don't think your unwraps will fail, but if they do, it's good to have a reminder of what's going on. Even if the expect never fails, the message provides additional documentation for those reading the code.

You'll like Rust, because concurrent mutation of a value is impossible if you hold a `&` or `&mut` to the value. So this can in fact be ruled out by the programmer.

It's only impossible in safe code. Unsafe cade can violate those rules all day long. You can't guarantee that there's no unsafe code running concurrently.

Any use of `unsafe` that breaks unrelated safe code is broken and buggy; if that scenario would happen like you describe it, the code is breaking Rust's aliasing rules: that's possible using `unsafe` but invalid and leads to UB.

I'm not talking about 'uses of unsafe', I'm talking about code that is unsafe. Much of that code is not even written in Rust, so there's no 'unsafe' to use.

Ok, so code that is memory unsafe (broken!). One must still say "unsafe" to bring it into Rust (to use ffi, or make a safe wrapper); so there is still a clear location in the Rust code that is to blame.

Concurrently modifying aliased memory (`&` references and pointers) is undefined behavior. Not just in Rust, but in just about any language.

As an aside, alias unsafety in Rust is always UB, even without concurrency.

> Now, this is a contrived example, and would better be written with `if let` in today's Rust, but this is the _kind_ of situation in which unwrap is totally, 100% cool, but the compiler can't know.

If the author knows it's safe, they should be able to express how they know in a way that the compiler can understand. Certainly I think there's a large space of use cases where the extra guarantee provided by forbidding unwrap would be well worth the cost of outlawing some "legitimate" cases, especially if we're just talking about doing so on a project level. (Though maybe they're not the rust target audience).

What if I want to have a reference to the last element in a vector that I just pushed? Without a push method that immediately returns such a reference, this will always involve an unwrap. And this is not just some weirdly constructed example, I've needed to do this in real code a couple of times already.

Pushing an element to a vector could return a guaranteed-non-empty vector. Admittedly it's unpleasant to write non-empty collections in a language that lacks HKT, since you have to reimplement a lot of stuff, but I'd consider that an argument for HKT rather than an argument for unwrap.

I can't reply to steveklabnik for some reason. But I think if let would replace that unwrap he uses there.

If you try to reply on HN too quickly, it hides the reply button as to discourage quick back-and-forths.

I mention in the post that this specific code would be best written with if let, but that it's not about the specifics, it's about the general pattern.

Right, it would likely use clippy but it would essentially be a --production target or profile, intended for builds where the binary will be run in production. And by 'and friends' I mean calls that panic for the same reason as unwrap, such as expect or ok.

The goal is to stop code from reaching production inadvertently, not to prevent all sources of panics.

But if we accept the premise that "it's acceptable in some cases to have unwrap() in code that targets 'production'" then it wouldn't make sense to have a production profile that bars its use. The word "production" is in the global namespace and I think you want something more specific to your use case.

Rather, one could define a rust coding guide for themselves that deems unwrap() inappropriate for production use. (and in that case use the lint Steve suggests).

Seems like you'd want a lint rule where if unwrap is used, it must have a comment preceding it (of some formal syntax) describing why it's necessary or appropriate. Thus the build can have all uses of unwrap known as explicitly allowed, allowing all the possible sites of panics to be enumerated and known, a useful property to have

In that case you may want "expect" over unwrap.

clippy has this already :)

For those not familiar with Clippy:

https://github.com/Manishearth/rust-clippy

It's a very useful tool; the main thing about it that annoys me is that it only works with nightly.

Reply on news.ycombinator.com