Option::infallible() - a good idea?

Do you think it would be a good idea to have infallible() along with unwrap() on Option and Result?

The rationale is that often you know that an operation will never fail. Yet there is no ergonomic way to convey that assumption to the reader of the code. unwrap() is often used as a quick and dirty workaround when you have an option and it is recomended in Rust Book to try to replace unwrap() with proper error handling. expect() on the other hand can convey the meaning but it's verbose.

Compare the options:

serde_json::to_string(&vec![1,2]).infallible()

serde_json::to_string(&vec![1,2]).expect("Always serializes correctly")

serde_json::to_string(&vec![1,2]).unwrap()
1 Like

If they both do the same thing, I don't see the point, it'll just be confusing. There is also the unsafe unwrap_unchecked, which will perform extra optimizations, but generally it's better to unwrap unless you are 101% percent sure.

expect() on the other hand can convey the meaning but it's verbose.

expect is meant to give extra context, it's only verbose if your message is verbose. infallible would do nothing more than unwrap, I don't see how it would be an improvement.

15 Likes

I agree in general, though on the otherhand, all of todo!(), unreachable!(), unimplemented!(), and panic!() exist. This is perhaps like going from panic!() to unreachable!(). Same effect, different…feel?

Usually I just use .expect() myself so that if my assumptions are ever wrong, at least something is guaranteed to come out of the woodwork in the error. Maybe if .infallible() to a string to explain why it is considered such, it would be more useful, but as merely an alternate .unwrap() spelling, meh (IMO).

3 Likes

Why would you want a string over a comment? If the code is unreachable, putting a string in the binary is a waste of memory.

1 Like

If it compiles down to unreachable!(string_given), then fine. Do unreachable!() arguments ever end up anywhere even in debug builds?

unreachable! just calls panic!, so I'd expect so. Hiding that behind Option::infallible would make no difference, because Option::unwrap already invokes panic! too.

But the alternative here is that you could just write

serde_json::to_string(&vec![1,2]).unwrap() // infallible

And that will compile down to nothing new, because comments are not included in executable code at all. They're also much more flexible and can communicate more subtle and important information than an .infallible call ever could. They're always exactly as verbose as you want them to be.

serde_json::to_string(&vec![1,2]).unwrap() // This is infallible because we returned above on `None`.

Maybe the closest thing in current Rust:

serde_json::to_string(&vec![1, 2]).unwrap_or_else(|_| unreachable!());
1 Like

Perhaps I'm being unclear, so allow me to rephrase my objection in a different form.

What intent is being clarified by this new function? The intent of calling unwrap on an option is that the Option is Some: calling unwrap on a None value results in an unfriendly error message from a panic indicating what happened. It is clear from this behavior that there is a bug in the program, and that bug is to be fixed by ensuring unwrap is not called on a None value.

Whet more is communicated by this new method? When is it wrong to use unwrap, where it would be correct to use infallible?

6 Likes

I guess I'm more interested in the state that, over time, the invariant holders and the invariant expecters migrate away from each other and then change. The message being in the error message helps to track things down again.

Again, I'm not really thrilled with the new method myself, but just wanted to observe that the difference is similar to that between panic!() and unreachable!().

But panic will indicate a misuse of an API, whereas unreachable is always a bug in the library. To avoid misuse of Option in an API, you would rewrite it to not use Option, and take the T directly. (As I said, the convention is already that unwrap indicates a bug. Trying to retcon unwrap as a non-bug won't work because all the current code already exists with a contrary meaning, and expect is right there for that.)

In other words, if I am to understand what you're saying, the new convention would be that one should call .infallible on options which must not be None, but .unwrap on ones which may be None even though they should never be? I do struggle to see a difference there. (I.e., If you wanted to panic on bad user input, you should probably just match None and give a decent panic message, rather than calling unwrap and showing them that garbage.)

1 Like

"Infallible" to me says something like Result<_, !>, where you convey the expectation that it's infallible by using into_ok to get the value out in a way that won't compile any more if the error type changes to something inhabited.

But that doesn't make sense for Option, since there's no type-level way to say it's not None. There could be into_none, that only works on Option<impl Into<!>>, but that feels mostly unhelpful.

I think that unwrap does a fine job of just communicating "I'm not expecting this to be None".

17 Likes

Plus, unwrap is already established. If someone sees infallible, they'll have to look it up, defeating the point.

1 Like

It is recommended in Rust Book to use .unwrap() if you know that an operation will never fail.

https://doc.rust-lang.org/stable/book/ch09-03-to-panic-or-not-to-panic.html#cases-in-which-you-have-more-information-than-the-compiler

6 Likes

I'm not happy with infallible name either. It's neither short nor particluarly clear to a new-commer.

The biggest issue I have with .unwrap() is that during development it makes sense to use unwrap initially, but then it is hard to discern which uses are infallible and which need to be replaced with proper error handling.

.expect("infallible")
.unwrap() // TODO: remove

Seem like resonable solutions. I only wish we had a standard way of doing this, so that I'm not inventing stuff others have to then learn and/or reinvent.

I have seen this proposed on the forum previously, as a kind of dependent types application, allowing the following to compile:

if let None = opt {
    return;
}
let Some(x) = opt; // cannot fail

It seems like Result::into_ok is about as far as that's going to get without dependent types though.

For my own part, I don't see .unwrap_or_else(|_| unreachable!()) often enough in my codebases to warrant inclusion in std.

I don't think this is a good idea. This really feels like the role of expect. If you think that an unwrap is infallible then you're supposed to put the explanation of why you never expect it to fail in the expect call. I'm pretty sure that's why it's called expect. I would not enjoy just being told that something wasn't supposed to fail because it was infallible, that's barely any more information than you get from an unwrap.

12 Likes

Also note that if the compiler can know that the Option really cannot be None or the Result cannot be Err(...), it can optimize the check out of .unwrap(), so that the generated code really does not execute a conditional (and the binary don't need to contain the error message, if that is true for all unwrap calls in the program).

(This probably requires inlining the function call that generates the Option / Result, and I'm guessing serde_json::to_string(&vec![1,2]) is probably too complex for this to happen.)

3 Likes

I guess the answer to the topic question is: Probably not a good idea.

Still It would be nice to have a clear convention on how to mark a logically infallible Option/Result. Currently it's a mix of "unwrap() in inffalible cases is ok" and "expect() should explain why this never fails".

(sorry for the edits)

expect is an easier to debug version of unwrap. Which one to use depends either on how certain you are that a failure will never occur (sometimes it is trivial) or on how you want to behave when your assumptions are wrong (sometimes you want to protect against changing conditions or future code changes, in which case expect is much better at acting like a runtime assertion.)

My conventions are that I always document why a failure can't occur, either as a comment on unwrap, or an expect argument. That choice usually depends on whether that non-failure determination is made by looking at the structure of the code, or the values given to me from outside, respectively.

3 Likes

depends on whether that non-failure determination is made by looking at the structure of the code, or the values given to me from outside, respectively.

That's a pretty good way to make the distinction. I like it.