5 ms·

Ugh. The basic argument is something like "all functions should be total", but the problem is that proving that a function is total is really annoying, so we l

Ugh. The basic argument is something like "all functions should be total", but the problem is that proving that a function is total is really annoying, so we live with two other consequences instead: 1. Some functions are not total, and you just have to know the preconditions. 2. Some functions are total, but the type system is not powerful enough to prove this (possibly because they call functions in category #1). The same discussion has been going on for decades in Haskell-land, and there's no definitive consensus, but my sense is that we'd rather tolerate a panic here and there rather than try and deal with the fuss of propagating errors or proving that our functions are total in the first place. For example, take a look at this function. I ask, > Is this a reasonable way to write this? struct IVec2(i32, i32); impl IVec2 { fn parse(d: [u8; 8]) -> Self { Vec2(i32::from_le_bytes(d[0..4].try_into().unwrap()), i32::from_le_bytes(d[4..8].try_into().unwrap())) } } Someone from the Rust community will come in and say, > Arguably, unwrap() is unreasonable. ...but why? The function is total. It won't panic. The response is, > ... an error should be properly handled ... There is no error, the code can't fail. I don't know of another way to write the function without a panic hidden somewhere, either in an unwrap() or inside an indexing operation. For some reason, some members of the Rust community think of this as a problem to be solved. It's not a problem. The unwrap() method is just too damn useful. It's too damn useful to be able to write functions that are not total. I'd say that the key here is coming up with a sensible style for when you are okay with panicking in your code, and then accepting the consequences of that decision. ("Total" means that the function returns a result for all inputs. Unwrap is not total. In Haskell, "head" is not total--it returns the first element of a list, but throws an exception if the list is empty.)

rowanG077 4y ago
I couldn't agree more. If course it would be nice of we could prove everything. It would even nicer if we would have a compiler that is smart enough. But it's just not practical for a lot of cases.
Diggsey 4y ago
I think the idea is that `expect()` accomplishes the same thing, but allows you to put an explanation of why the function should be total (such that if it does indeed panic, you can see what went wrong with the reasoning). That said, I do think there's a place for "unwrap" in eg. tests.
- dietrichepp 4y ago
  That seems even more verbose, for a simple function that is already very verbose to begin with, given how little the function does.
- ericbarrett 4y ago
  I find 9 times out of 10, the source location of the panic (main.rs:123) is enough for debugging, and the .expect() text is redundant. I would never advocate .unwrap() for production services where other people may have to debug the code years after you write it, but there are many, many other use cases for Rust.
jacobr1 4y ago
We might not be able to always reason about function totality, but perhaps sometimes we can. Isn't that what `std::convert::Infallible` is trying to do? So you get fn try_from(value: U) -> Result<Self, Infallible> In which case you could potentially avoid the need for an unwrap.
- dietrichepp 4y ago
  Could you give a more complete example? I see the type signature for the function you're proposing, but I still don't understand what is supposed to replace .unwrap() on the inside of the function.
  - rowanG077 4y ago
    The "fix" would be to return a Result. Which is a sum of an error and the type you want to return.
    - dietrichepp 4y ago
      You can't return an Infallible, though, because it has no variants. You still need to do something at the point of unwrap().
    - Macha 4y ago
      But again, the function cannot fail. It could return a Result<Vec2, Infallible>, I guess, but then you're just pushing the unwrap to caller without any need to.
      - rowanG077 4y ago
        That's the entire point of the OP. That it can't fail so you might as well call unwrap.
  - game-of-throws 4y ago
    You'd pair it with something like this https://docs.rs/unwrap-infallible/ https://docs.rs/unwrap-infallible/
dymk 4y ago
If we truly want to get rid of unwrap() in most cases, and not have the language be a bear* to use, then it needs a tactic language or something for proving invariants like that at compile time. Past that, we're going to have code that calls unwrap(), unless we want what should be simple to consist mostly of pattern matching boilerplate (and functions returning Result<T,...> when they could just be T). * - Of course, now you have to deal with a tactic language / dependent types, and those could arguably be called a bear in and of themselves ;)
brundolf 4y ago
The standard method name also makes it super easy to go back and tighten things up when the time comes I recently had a nontrivial project where I'd used maybe a dozen unwrap()s in different places and had gotten a handful of bug reports for resulting panics. So I decided to make a pass and finally clean them up Project-wide search for unwrap(), went through and properly handled all of them, very quickly and easily plugged all the holes, no more panics being reported
- echelon 4y ago
  It's also something you can do static analysis for with tools, and I imagine the compiler can be adjusted to warn or disallow its use in modules and dependencies.
proto_lambda 4y ago
There is a problem here - it's verbose and prone to silent-ish breakage if the surrounding context changes e.g. due to refactors (well, probably not with this tiny example, but I hope you know what I mean). Thankfully, there is also a solution to this problem: https://github.com/rust-lang/rust/issues/90091 https://github.com/rust-lang/rust/issues/90091 split_array will eventually allow something like the following, which avoids any explicit indexing and will fail to compile if the sizes don't match up: struct IVec2(i32, i32); impl IVec2 { fn parse(b: [u8; 8]) -> Self { let (first, second) = b.split_array(); IVec2(i32::from_le_bytes(first), i32::from_le_bytes(second)) } }
- skrtskrt 4y ago
  I do appreciate that Rust tends to have the mentality that they will, after careful consideration, add something like split_array to solve X ugly problem. But it leads to just a TON of features and approaches to learn.
- dietrichepp 4y ago
  > [...] well, probably not with this tiny example, but I hope you know what I mean [...] Well, here's the thing. These small pieces of code DO show up in actual, real codebases! I've gotten feedback during code reviews along the lines of, "What if the surrounding context is refactored, this tiny, simple piece code could break" and to be perfectly honest, when I get it, I come down to your desk and we have a discussion about whether that kind of feedback is appropriate, and the purpose of code reviews. (It also always seems like the issue I'm looking at is "fixed in nightly", but some of those features in nightly take a long time to get accepted, and the ergonomics of split_array() seem a bit dubious to me. Are you going to chain three split_array() for four fields? Obviously, for simple serialization and deserialization there are a ton of different options to automate this code away, but it's nice to be able to write simple code like this when appropriate, and the ergonomics of simple tasks matter.)
  - joshlemer 4y ago

rowanG077 4y ago

I couldn't agree more. If course it would be nice of we could prove everything. It would even nicer if we would have a compiler that is smart enough. But it's just not practical for a lot of cases.

Diggsey 4y ago

I think the idea is that `expect()` accomplishes the same thing, but allows you to put an explanation of why the function should be total (such that if it does indeed panic, you can see what went wrong with the reasoning). That said, I do think there's a place for "unwrap" in eg. tests.

dietrichepp 4y ago
That seems even more verbose, for a simple function that is already very verbose to begin with, given how little the function does.
ericbarrett 4y ago
I find 9 times out of 10, the source location of the panic (main.rs:123) is enough for debugging, and the .expect() text is redundant. I would never advocate .unwrap() for production services where other people may have to debug the code years after you write it, but there are many, many other use cases for Rust.

jacobr1 4y ago

We might not be able to always reason about function totality, but perhaps sometimes we can. Isn't that what `std::convert::Infallible` is trying to do? So you get fn try_from(value: U) -> Result<Self, Infallible> In which case you could potentially avoid the need for an unwrap.

dietrichepp 4y ago
Could you give a more complete example? I see the type signature for the function you're proposing, but I still don't understand what is supposed to replace .unwrap() on the inside of the function.
- rowanG077 4y ago
  The "fix" would be to return a Result. Which is a sum of an error and the type you want to return.
  - dietrichepp 4y ago
    You can't return an Infallible, though, because it has no variants. You still need to do something at the point of unwrap().
  - Macha 4y ago
    But again, the function cannot fail. It could return a Result<Vec2, Infallible>, I guess, but then you're just pushing the unwrap to caller without any need to.
    - rowanG077 4y ago
      That's the entire point of the OP. That it can't fail so you might as well call unwrap.
- game-of-throws 4y ago
  You'd pair it with something like this https://docs.rs/unwrap-infallible/ https://docs.rs/unwrap-infallible/

dymk 4y ago

If we truly want to get rid of unwrap() in most cases, and not have the language be a bear* to use, then it needs a tactic language or something for proving invariants like that at compile time. Past that, we're going to have code that calls unwrap(), unless we want what should be simple to consist mostly of pattern matching boilerplate (and functions returning Result<T,...> when they could just be T). * - Of course, now you have to deal with a tactic language / dependent types, and those could arguably be called a bear in and of themselves ;)

brundolf 4y ago

The standard method name also makes it super easy to go back and tighten things up when the time comes I recently had a nontrivial project where I'd used maybe a dozen unwrap()s in different places and had gotten a handful of bug reports for resulting panics. So I decided to make a pass and finally clean them up Project-wide search for unwrap(), went through and properly handled all of them, very quickly and easily plugged all the holes, no more panics being reported

echelon 4y ago
It's also something you can do static analysis for with tools, and I imagine the compiler can be adjusted to warn or disallow its use in modules and dependencies.

proto_lambda 4y ago

There is a problem here - it's verbose and prone to silent-ish breakage if the surrounding context changes e.g. due to refactors (well, probably not with this tiny example, but I hope you know what I mean). Thankfully, there is also a solution to this problem: https://github.com/rust-lang/rust/issues/90091 https://github.com/rust-lang/rust/issues/90091 split_array will eventually allow something like the following, which avoids any explicit indexing and will fail to compile if the sizes don't match up: struct IVec2(i32, i32); impl IVec2 { fn parse(b: [u8; 8]) -> Self { let (first, second) = b.split_array(); IVec2(i32::from_le_bytes(first), i32::from_le_bytes(second)) } }

skrtskrt 4y ago
I do appreciate that Rust tends to have the mentality that they will, after careful consideration, add something like split_array to solve X ugly problem. But it leads to just a TON of features and approaches to learn.
dietrichepp 4y ago
> [...] well, probably not with this tiny example, but I hope you know what I mean [...] Well, here's the thing. These small pieces of code DO show up in actual, real codebases! I've gotten feedback during code reviews along the lines of, "What if the surrounding context is refactored, this tiny, simple piece code could break" and to be perfectly honest, when I get it, I come down to your desk and we have a discussion about whether that kind of feedback is appropriate, and the purpose of code reviews. (It also always seems like the issue I'm looking at is "fixed in nightly", but some of those features in nightly take a long time to get accepted, and the ergonomics of split_array() seem a bit dubious to me. Are you going to chain three split_array() for four fields? Obviously, for simple serialization and deserialization there are a ton of different options to automate this code away, but it's nice to be able to write simple code like this when appropriate, and the ergonomics of simple tasks matter.)
- joshlemer 4y ago