It also makes it very obvious in the code, something very dangerous is happening here. As a code reviewer you should see an unwrap() and have alarm bells going off. While in other languages, critical errors are a lot more hidden.

I hate that it's a method. That can get lost in a method chain easily enough during a code review.

A function or a keyword would interrupt that and make it less tempting

Well, you can request Clippy to tell you about them. I do that in my hobby projects.

> What was wrong was the developer assumption, not the use of unwrap.

How many times can you truly prove that an `unwrap()` is correct and that you also need that performance edge?

Ignoring the performance aspect that often comes from a hat-trick, to prove such a thing you need to be wary of the inner workings of a call giving you a `Return`. That knowledge is only valid at the time of writing your `unwrap()`, but won't necessarily hold later.

Also, aren't you implicitly forcing whoever changes the function to check for every smartass dev that decided to `unwrap` at their callsite? That's bonkers.

I doubt that this unwrap was added for performance reasons; I suspect it was rather added because the developer temporarily didn't want to deal with what they thought was an unlikely error case while they were working on something else; and no other system recognized that the unwrap was left in and flagged it before it was deployed on production servers.

If I were Cloudflare I would immediately audit the codebase for all uses of unwrap (or similar rust panic idioms like expect), ensure that they are either removed or clearly documented as to why it's worth crashing the program there, and then add a linter to their CI system that will fire if anyone tries to check in a new commit with unwrap in it.

Panics are for unexpected error conditions, like your caller passed you garbage. Results are for expected errors, like your caller passed you something but it's your job to tell if it's garbage.

So the point of unwrap() is not to prove anything. Like an assertion it indicates a precondition of the function that the implementer cannot uphold. That's not to say unwrap() can't be used incorrectly. Just that it's a valid thing to do in your code.

Note that none of this is about performance.

> when you do unwrap() you're saying the Result should NEVER fail

Returning a Result by definition means the method can fail.

> Returning a Result by definition means the method can fail.

No more than returning an int by definition means the method can return -2.

> No more than returning an int by definition means the method can return -2.

What? Returning an int does in fact mean that the method can return -2. I have no idea what your argument is with this, because you seem to be disagreeing with the person while actually agreeing with them.

> What? Returning an int does in fact mean that the method can return -2.

What? No it doesn't.

  fn square(n: i32) -> i32 {
      n * n
  }

This method cannot return -2.

Though in this case it's more like knowing that the specific way you call the function in foo.rs will never get back a -2.

  fn bar(n: i32, allow_negative: bool) -> i32 {
      let new = n * 2;
      if allow_negative || new >= 0 { new } else { 0 }
  }
  bar(x, false)

What? Results have a limited number of possible error states that are well defined.

Some call points to a function that returns a Result will never return an Error.

Some call points to a function that returns an int will never return -2.

Sometimes you know things the type system does not know.

The difference is functions which return Result have explicitly chosen to return a Result because they can fail. Sure, it might not fail in the current implementation and/or configuration, but that could change later and you might not know until it causes problems. The type system is there to help you - why ignore it?

Because it would be a huge hassle to go into that library and write an alternate version that doesn't return a Result. So you're stuck with the type system being wrong in some way. You can add error-handling code upfront but it will be dead code at that point in time, which is also not good.

As a hypothetical example, when making a regex, I call `Regex::new(r"/d+")` which returns a result because my regex could be malformed and it could miscompile. It is entirely reasonable to unwrap this, though, as I will find out pretty quickly that it works or fails once I test the program.

Yeah, I think I expressed wrongly here. A more correct version would be: "when you do unwrap() you're saying that an error on this particular path shouldn't be recoverable and we should fail-safe."

Division can fail, but I can write a program where I'm sure division will not fail.