# Pre-RFC: Deprecate ok\_or method

**URL:** <https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681>\
**Category:** language design\
**Created:** [June 2, 2018, 4:02pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681 "2018-06-02T16:02:23Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\
**Post date:** [June 2, 2018, 4:02pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/1 "2018-06-02T16:02:23Z")

</div>

## Motivation

There is plenty of issues when people unintendely use the eager version of mapping result method just because it saves some typing. It results in [confusion among the community](https://users.rust-lang.org/t/performance-impact-of-result-vs-option/17783). It happens even with the [compiler team itself](https://github.com/rust-lang/rust/pull/50051).

The most popular using of this method is something like following:

```rust
let path = try_get_path(my_str).ok_or(MyError::InvalidPath(my_str))?;

```

The problem here is that `MyError` struct is created not only if error actually happened, but on happy path too.

In most cases, people really want to write

```rust
let path = try_get_path(my_str).ok_or_else(|| MyError::InvalidPath(my_str))?;

```

But they don’t use it, mostly because it requires a bit more typing as well as they might not notice the difference. Of course, if they find a significant slowdown, they could benchmark code, spot these places and fix them, but in most cases it would remain unnoticed (although it affects performance drastically).

Rust is going into the new epoch, so I believe it’s a good time to consider the change.

## The proposal is:

1. Deprecate `ok_or` method on `Option` type. We already have a lint in clippy that suggest to replace it with `ok_or_else` if any computations are done there. It’s a good signal that it shall be done. On the other hand, `or_or_else` with constant value lambda would be always inlined and thus have same performance as before.
2. _(Optional)_ Replace `or_or` method via `or_or_else` signature and implementation, remove the latter (or make it just the proxy to the `ok_or`).

I believe that we mostly want a lazy evaluation rather than eager one. `Error` value is never a constant, except when it’s just “something bad happened” static string. So its recalculation on happy paths seems to be a undesirable behavior. In the rare cases when it’s required compiler could always figure out that constant function may be inlined and replaced by value.

This change serves two goals: remove a possible pitfall and make standard library more consistend so user don’t have to choose between two similar methods, just pick one, and if it may be optimized compiler will do so.

---

<div class="post-metadata">

**Author:** ![steveklabnik](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/steveklabnik/32/4524_2.png) [@steveklabnik](https://internals.rust-lang.org/u/steveklabnik)\
**Post date:** [June 2, 2018, 4:21pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/2 "2018-06-02T16:21:43Z")

</div>

We can only deprecate, but we cannot remove or replace it. Epochs can’t make this kind of change.

---

<div class="post-metadata">

**Author:** ![sfackler](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/sfackler/32/9_2.png) [@sfackler](https://internals.rust-lang.org/u/sfackler)\
**Post date:** [June 2, 2018, 4:52pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/4 "2018-06-02T16:52:44Z")

</div>

Why is this specifically talking about only ok\_or and not other similar functions?

I don’t agree that we need to remove a method because people have accidentally misused it. The fact that arguments are evaluated before a call to a function or method is pretty fundamental to ~all imperative languages.

---

<div class="post-metadata">

**Author:** ![leodasvacas](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/leodasvacas/32/4060_2.png) [@leodasvacas](https://internals.rust-lang.org/u/leodasvacas)\
**Post date:** [June 2, 2018, 5:02pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/5 "2018-06-02T17:02:22Z")

</div>

> The most popular using of this method

I'll need more data to be convinced that most uses of `ok_or` dynamically allocate a string. It is very common for errors to simply be constants. Also that method is part of an API convention, we also have `map_or` and `unwrap_or`. You make valid points for when we should prefer `ok_or_else`, but in many cases `ok_or` is more readable and arguably more efficient, so I see nothing intrinsically wrong with it.

This does give me the idea of having an `.ok_or!(foo)`, when we get macros in method position, that desugars into an inlined `ok_or`, gaining us the best of both worlds. That would be cool.

---

<div class="post-metadata">

**Author:** ![H2CO3](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/h2co3/32/2849_2.png) [@H2CO3](https://internals.rust-lang.org/u/H2CO3)\
**Post date:** [June 2, 2018, 6:04pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/6 "2018-06-02T18:04:01Z")

</div>

If the issue is frequent misuse (which I’m not convinced about) of the eager methods when the lazy ones should be used instead, that is more of a cultural/educational issue, not a technical one. I think that docs are already pretty clear, but perhaps we could educate people more clearly? It’s also possible to write a Clippy lint that checks for the misuse of `ok_or`.

---

<div class="post-metadata">

**Author:** ![birkenfeld](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/birkenfeld/32/1150_2.png) [@birkenfeld](https://internals.rust-lang.org/u/birkenfeld)\
**Post date:** [June 2, 2018, 6:39pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/7 "2018-06-02T18:39:16Z")

</div>

> [@H2CO3](#):
>
> It’s also possible to write a Clippy lint that checks for the misuse of `ok_or` .

There already is 🙂

---

<div class="post-metadata">

**Author:** ![H2CO3](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/h2co3/32/2849_2.png) [@H2CO3](https://internals.rust-lang.org/u/H2CO3)\
**Post date:** [June 2, 2018, 6:54pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/8 "2018-06-02T18:54:16Z")

</div>

(Thanks, I tried to check that before commenting, but I couldn’t find it…)

---

<div class="post-metadata">

**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\
**Post date:** [June 2, 2018, 7:13pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/9 "2018-06-02T19:13:24Z")

</div>

It's written in the proposal itself, if you read it 🙂

> I don’t agree that we need to remove a method because people have accidentally misused it. The fact that arguments are evaluated before a call to a function or method is pretty fundamental to ~all imperative languages.

If people misuse it on a day to day basis then it should be receive more attention, I do believe. The fact that there is a lint in clippy is already a signal. Probably, it's enough, probably it's not. This is the reason why I have written this.

---

<div class="post-metadata">

**Author:** ![H2CO3](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/h2co3/32/2849_2.png) [@H2CO3](https://internals.rust-lang.org/u/H2CO3)\
**Post date:** [June 2, 2018, 7:32pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/10 "2018-06-02T19:32:33Z")

</div>

> [@Pzixel](#):
>
> people misuse it on a day to day basis […] The fact that there is a lint in clippy is already a signal

But… they don't? Clippy has plenty of lints against accidental mistakes which are rare in practice, just as an extra precautionary measure. Clippy doesn't only include lints because they are catastrophic or because everyone makes those mistakes with every second keystroke. The purpose of a linter is to squeeze out even the last bit of code quality if it can. By this logic we could argue for the removal of `match` expressions, the `clone` method or references, because there are several lints against those too.

As you probably noticed, I am very much against anything in Rust (especially anything additive) that has a realistic chance of resulting in generally poorer, buggier code. The `ok_or` method is just not an issue in practice, though.

---

<div class="post-metadata">

**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\
**Post date:** [June 2, 2018, 8:10pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/11 "2018-06-02T20:10:06Z")

</div>

> [@H2CO3](#):
>
> But… they don’t? Clippy has plenty of lints against accidental mistakes which are rare in practice, just as an extra precautionary measure.

Well, I think all these lints should be considered as possible language fix. You see lint, you check that everybody makes this mistake, and you have two options: "Hmm, we must follow the principle of least astonishment and fix it in the language" or "it's a rare case and docs are clear about this behavior". I just think that it's the former case, so I have written this proposal. If I'm alone thinking it that it's ok, we're just discussing.

I see that it's coherent with "map\_or", "unwrap\_or" and others, but probably we could reexamine if they are really required. Maybe not. It's discutable.

> By this logic we could argue for the removal of match expressions, the clone method or references, because there are several lints against those too.

No, it's not implications of this logic, it's _ad absurdum_ argument. ∃ redundancy != ∀x x == redundancy

---

<div class="post-metadata">

**Author:** ![Ixrec](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ixrec/32/6754_2.png) [@Ixrec](https://internals.rust-lang.org/u/Ixrec)\
**Post date:** [June 2, 2018, 8:23pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/12 "2018-06-02T20:23:59Z")

</div>

> you have two options: “Hmm, we must follow the principle of least astonishment and fix it in the language” or “it’s a rare case and docs are clear about this behavior”.

This statement assumes that it's always both possible and unproblematic to "fix it in the language". In this case, we're talking about detecting whether the (arbitrary) expression passed to ok\_or will perform some computationally expensive task that would be better off done lazily. That's obviously not something we can ever hope to solve for in the general case (the closest we've seen might be some of the "effect system" proposals to track whether a function dynamically allocates, but so far those are all very heavyweight and dubious proposals and that's still only a subset of the problem). What we clearly can do is detect some very specific special cases of it that happen to be commonly written _and_ are usually better off as ok\_or\_else, which is precisely what makes a lint appropriate even if those special cases are extremely common in practice.

---

<div class="post-metadata">

**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\
**Post date:** [June 2, 2018, 8:31pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/13 "2018-06-02T20:31:52Z")

</div>

> [@Ixrec](#):
>
> This statement assumes that it’s always both possible and unproblematic to “fix it in the language”.

I don't understand where you get it. This statement clearly says that you have an option to embrace a lint on language level, or not. That's it. It doesn't say anything about "unproblematic" of anything.

> [@Ixrec](#):
>
> In this case, we’re talking about detecting whether the (arbitrary) expression passed to ok\_or will perform some computationally expensive task that would be better off done lazily. That’s obviously not something we can ever hope to solve for in the general case (the closest we’ve seen might be some of the “effect system” proposals to track whether a function dynamically allocates, but so far those are all very heavyweight and dubious proposals and that’s still only a subset of the problem).

No, it's easy: just make everything lambda. If it's a simple constant expression then it would get inlined by optimizer. If it's not, then it shouldn't be computed eagerly. Nothing about any specific case. Optimizer already knows what's constant expression is.

---

<div class="post-metadata">

**Author:** ![kornel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kornel/32/2711_2.png) [@kornel](https://internals.rust-lang.org/u/kornel)\
**Post date:** [June 3, 2018, 12:48am UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/14 "2018-06-03T00:48:00Z")

</div>

I’m not keen on this. It is convenient when used properly, and there’s a Clippy lint for misuse.

---

<div class="post-metadata">

**Author:** ![MajorBreakfast](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/majorbreakfast/32/4015_2.png) [@MajorBreakfast](https://internals.rust-lang.org/u/MajorBreakfast)\
**Post date:** [June 3, 2018, 7:02am UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/15 "2018-06-03T07:02:52Z")

</div>

I think this should be deprecated. I’m not so keen on footguns in std.

---

<div class="post-metadata">

**Author:** ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)\
**Post date:** [June 3, 2018, 4:24pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/16 "2018-06-03T16:24:10Z")

</div>

Things like `.unwrap_or(0)` are perfectly reasonable. Forcing that to be `.unwrap_or_else(|| 0)` every time is _way_ overkill.

Having both optimistic (lint if contains unknown function calls, could be warn-by-default) and pessimistic (lint on anything but trivial things like literals, probably allow-by-default) clippy lints, however, does seem reasonable.

---

<div class="post-metadata">

**Author:** ![vorner](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/vorner/32/3310_2.png) [@vorner](https://internals.rust-lang.org/u/vorner)\
**Post date:** [June 3, 2018, 4:43pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/17 "2018-06-03T16:43:15Z")

</div>

I think that if it was such a common problem, then the first step could be not deprecating the method outright, but promoting the warning from clippy lint to one in rustc. But only on the ones that contain a function call. Furthermore, rustc is probably in a better position than clippy (well, I just guess here) to know if the thing inside contains some computation or if it is just inlined as a constant (eg. `Some(42)` is probably not a call).

I would go for more extreme ways if that doesn’t help. I personally find a lot of legitimate uses of `unwrap_or` or `ok_or`.

However, I think we should recommend clippy more to beginners. It is useful.

---

<div class="post-metadata">

**Author:** ![xfix](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/xfix/32/2726_2.png) [@xfix](https://internals.rust-lang.org/u/xfix)\
**Post date:** [June 3, 2018, 7:11pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/18 "2018-06-03T19:11:43Z")

</div>

> [@Pzixel](#):
>
> ```rust
> let path = try_get_path(my_str).ok_or(MyError::InvalidPath(my_str))?;
> 
> ```
> 
> The problem here is that `MyError` struct is created not only if error actually happened, but on happy path too.

If `InvalidPath` is an enum variant (not a regular function call), then there isn't really a cost in doing so, as the optimizer should be able to realize there are no side-effects.

---

<div class="post-metadata">

**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\
**Post date:** [June 4, 2018, 8:02am UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/19 "2018-06-04T08:02:04Z")

</div>

> [@scottmcm](#):
>
> Things like `.unwrap_or(0)` are perfectly reasonable. Forcing that to be `.unwrap_or_else(|| 0)` every time is _way_ overkill.

Nobody proposes to keep `or_else` variants and deprecate all their counterparts. The question was do we really need `ok_or_else` to be just coherent with others `or_else`s?

> [@vorner](#):
>
> I think that if it was such a common problem, then the first step could be not deprecating the method outright, but promoting the warning from clippy lint to one in rustc. But only on the ones that contain a function call. Furthermore, rustc is probably in a better position than clippy (well, I just guess here) to know if the thing inside contains some computation or if it is just inlined as a constant (eg. `Some(42)` is probably not a call).

That's a valid option too. And it look more consistent to me. I really didn't like the idea of deprecating such a basic thing, but it's really a footgun in most cases. Lint like `deny(heavy_or)` would also help with other `or_else`s, however, they tend to use constants indeed, unlike `ok_or` which almost always calls some methods.

---

<div class="post-metadata">

**Author:** ![kornel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kornel/32/2711_2.png) [@kornel](https://internals.rust-lang.org/u/kornel)\
**Post date:** [June 4, 2018, 12:28pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/20 "2018-06-04T12:28:06Z")

</div>

> [@Pzixel](#):
>
> unlike `ok_or` which almost always calls some methods.

That doesn't match what I've seen, which is almost exclusively use of enum variants:

```
thing_returning_option().ok_or(MyErrorEnum::ThingDidntWork)?;

```

---

<div class="post-metadata">

**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\
**Post date:** [June 4, 2018, 2:21pm UTC](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681/21 "2018-06-04T14:21:11Z")

</div>

> [@kornel](#):
>
> That doesn’t match what I’ve seen, which is almost exclusively use of enum variants:

Do you really think so? Because I mostly see something like:

```
fn path_to_cstring<P: AsRef<Path>>(path: P) -> Result<CString, failure::Error> {
    let path = path.as_ref();
    let x = path.to_str().ok_or(CvError::InvalidPath(path.into()))?;
    let result = CString::new(x)?;
    Ok(result)
}

```

_(I took this example from my own code)_

So I have mistaken just as described in my post. So I don't think it's really this rare as you describe it.

[Next page](https://internals.rust-lang.org/t/pre-rfc-deprecate-ok-or-method/7681.md?page=2)