9 ms·
Rust should not have provided `unwrap`
- boardwaalk 4y agoYes, we should have unwrap. Rust would move towards insufferable if it did not — sort of like Go and its “you must use every variable” thing. The way forward is a feature allows static checking of whether code paths could possibly panic so people can gradually mark their code as panic-free.
- pdimitar 4y agoAs I said in the past, I'd love a a compiler flag / Cargo.toml setting that disables all panicky Rust stdlib API. Obviously many people don't need or want that so it should be opt in only for those that do.
- user249 4y agoWhen I was learning Rust, unwrap felt like cheating
- grumpyprole 4y agoIt is! :) Perhaps the name could have made it more obvious: e.g. "get_or_fail"
- grumpyprole 4y agoUnwrap is just the same (or better) than expect(""). The cat is out the bag with expect. What is needed, is good documentation, style guides, example code etc to guide users towards understanding the trade-offs and making informed decisions. I suppose unwrap could have had a more scary name.
- azdavis 4y agoYou can enforce this with a clippy lint: https://rust-lang.github.io/rust-clippy/rust-1.62.0/index.html#disallowed_methods https://rust-lang.github.io/rust-clippy/rust-1.62.0/index.ht...
- deleted 4y ago[deleted]
- moffkalast 4y ago"Looks like you're having problems enforcing no unwrap. Would you like help? [Yes][No]"
- thaneross 4y agoThere's also this less generic lint: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used https://rust-lang.github.io/rust-clippy/master/index.html#un...
- lacker 4y agoI feel like the author is only thinking about high-quality production code. Often you're just writing a 30-line snippet of throwaway code to test something out. And then unwrap() is a bit nicer than expect("").
- paulgb 4y agoMy thoughts exactly. Without panic people (myself included) would just write expect("") when we want the behavior of unwrap(). I have some Rust code that I'm the only user of; in those cases I'd rather investigate an error when it comes up than spend time upfront thinking about errors that may never happen.
- stouset 4y agoFor me, the problem isn't `unwrap` vs. `expect`, it's that I can't statically guarantee that particularly mission-critical code never panics. I can be as careful as I want in writing panic-safe code, but there's no ergonomic and non-hacky way to ensure that I don't use third-party crates in a way that can possibly panic either.
- nimih 4y ago> I can be as careful as I want in writing panic-safe code, but there's no ergonomic and non-hacky way to ensure that I don't use third-party crates in a way that can possibly panic either. There's also no way to ensure those third-party crates don't enter an infinite loop; at some point, if you need to guarantee your program doesn't diverge, you need to either trust the libraries you're using to behave reasonably, or you need to audit them yourself. Removing `unwrap` wouldn't solve that problem, and given how easy it is to lint for, it doesn't even meaningfully contribute to its difficulty. As the GP and some of the other top level comments point out, `unwrap` is a genuinely useful tool to have in the std lib, so advocating for its removal (or that it was a bad choice to begin with, presumably hoping that future languages will avoid including amenities such as `unwrap`) seems like a poor design trade-off from where I stand.
- stouset 4y ago
- smoldesu 4y agoI feel mixed on this, it's kinda like arguing that the Hello World for Java needs to include classes because it starts people off on the right foot. Unwrap is admittedly a stupid tool, and if you're starting off in Rust it becomes one of those "when all you have is a hammer, everything looks like a nail" situations. It's like a can-opener for fussy functions that want you to write semantic code, and it's admittedly quite good at what it does. Too good. Like the author highlights, simply appending '?' to the end of your line is a better way to handle 90% of unwrap cases, and the 'panic' macro should cover most of everything else. Unwrap should really only be used when you disagree with the library developer about error handling. In all other cases, you should probably respect the error propagation of the program. Explaining all that to a new Rust user is hard though, so I don't really nitpick with it until people are comfortable with the memory model and control flow of Rust.
- Arcuru 4y agoClippy has a lint that will check for `.unwrap()` calls; https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used https://rust-lang.github.io/rust-clippy/master/index.html#un.... It is allowed by default, but you can turn it on in the configuration file.
- Cyph0n 4y agoIt’s still useful in cases where you’ve already handled the error case. For example: if res.is_err() { // Do something return; } let res = res.unwrap();
- JoshMcguigan 4y agoYou can replace this with a match statement to avoid the unwrap.
- Macha 4y agoIt does introduce another layer of nesting in the code, which can be undesirable from a readability perspective at times
- johntb86 4y agoRust is getting let-else to solve that problem: https://rust-lang.github.io/rfcs/3137-let-else.html https://rust-lang.github.io/rfcs/3137-let-else.html
- kannanvijayan 4y agoNot necessarily, you can extract the inner type by using the match as an expression, e.g.: ``` let val = match res { Err(e) => { ... return ...; } Ok(v) => v, }; // Use val without unwrapping here. ``` That said, even though I'd avoid unwrap in this particular case in my code, I generally disagree with the author. Taking out unwrap and leaving in expect would simply lead to a bunch of code that goes `let val = result.expect("!");`, which is equivalently bad when held up against the criticisms the author makes of unwrap. Unwrap isn't really harmful so much as a symptom and an escape valve around the fact that our type systems really aren't powerful enough to derive all the invariants the programmer can about their code.
- dymk 4y ago`unwrap` is fine. As the author mentions, it can be converted into `?` without much hassle if the function return type can be changed easily. And `?` looks cleaner than `unwrap`, so the incentives are aligned to nudge developers towards that instead. `unwrap` is plenty explicit, and developers know exactly what it does (and that it might panic) once they've spent a few hours with Rust. It's not the subtle footgun the author is making it out to be.
- oxff 4y agoSometimes you don't want to use `?` though, not every function is made to return a Result / Option.
- dymk 4y agoIn that case, you use `unwrap`, or you manually convert it into your return type, or you provide an `impl Try` [0] for your return type once that feature becomes stable (already possible if you're on nightly) [0] - https://github.com/rust-lang/rust/issues/84277 https://github.com/rust-lang/rust/issues/84277
- mcronce 4y agoIf your function is fallible, your function is fallible - it's up to you to decide whether that means returning Result or panicking. If you want your function to be infallible, while itself calling a fallible function, you have to handle the error cases from your callees - you have `.unwrap_or_default()`, `.unwrap_or_else()`, `.unwrap_or()`, and simple `match` all in your toolbox to do this.
- throwaway234232 4y agoThe footgun here is that `unwrap` may be used inappropriately, and from what I see using github code search it's used inappropriately quite often.
- dymk 4y agoSo is `panic`, or the subtraction operator on unsigned values which may cause underflow.
- caylus 4y ago> Well, that puts regex compilation in the same category as array indexing in my mind, and means that the default regex compilation function should panic on the user’s behalf I like the Go stdlib idiom for this: many functions have a version with a "Must" prefix that has the behavior of panicking rather than returning an error. e.g. for regexp: https://pkg.go.dev/regexp#MustCompile https://pkg.go.dev/regexp#MustCompile
- masklinn 4y agoSo an ad-hoc way to do the exact same thing requiring doubling the size of your api? Is your problem with unwrap that it’s too easy to find?
- ianpurton 4y agoI'd also be happy if it was removed. It's too easy to use it rather than going up the learning curve of ?. ? Is one of the main benefits of rust.
- LegionMammal978 4y agoWhat about extremely trivial expected conditions that occur all over the program? "expect()" would get tiring pretty quickly. Regex::new(r"\bstruct\b").unwrap() // vs. Regex::new(r"\bstruct\b").expect("bad regex string literal")
- XorNot 4y agoIn this case at least it seems like one solution is just compile-time function evaluation? Due to the literal, hypothetically that Regex::new's result can be pre-computed and the result encoded in the output binary, not the initialization.
- lilyball 4y agoRegex used to offer a regex!() macro that did compile-time compilation, but this was removed a while back as it was too much of a maintenance burden and was getting in the way of performance optimization.
- mcguire 4y agoWell, there we go, then. Nobody wants to do the work.
- lilyball 4y agoIIRC it required basically two separate implementations of the data structures, one that could be constructed at compile-time and was less efficient than the one that could be built at runtime. And the need to support both was becoming a problem. I feel like there should be a middle ground here, a macro that validates at compile-time but still constructs at runtime. Hopefully the parser could still be shared between the macro and the runtime to ensure the macro can't get out of sync.
- deepsun 4y agoSounds very similar to annoying Java checked exceptions for stuff like 'new URL("http://example.com http://example.com")' -- it can't fail, but you have to catch it. The industry dealt with that by using your own utility functions like "create an Url and convert exceptions to runtime ones" (Java folks don't focus that much on quick pet projects, so don't mind extra work).
- Macha 4y agoI tend to agree with preferring alternatives (?, Expect, unwrap_or_else) to unwrap in most cases, but I do still find myself using unwrap in cases where it's basically impossible for it to fail. e.g. imagine a parse method that returns a Result in case the input is not a valid whatever, but I've just called it with a literal value. At the same time, looking at one of my codebase, 90% of such cases are in my test code
- mcronce 4y agoI agree with you. My general rule of thumb is that I use `unwrap()` for cases where the panic should be unreachable, and is at least reasonably easy to prove it's unreachable. Otherwise, `?` in 90% of code, with panics far up the stack, e.g. in `main()`
- masklinn 4y ago> My general rule of thumb is that I use `unwrap()` for cases where the panic should be unreachable, and is at least reasonably easy to prove it's unreachable. I don’t agree with that. To me unwrap straddles the line between “I don’t care to think about this right now” and “I don’t care about that case”. If I can prove it does not happen then it get an “expect” with a justification. Or even a map_err(|_| unreachable!(reasoning)) if I’m feeling really fancy. unwrap acts as a todo, I can grep for it and if I see one I know it’s something to fix.
- wongarsu 4y agoIn the Objections section the author concedes that some API calls most commonly fail on logic errors, like creating a regex, and writing an error message for each of these would be unnecessarily verbose. Their counter to this argument is that there should be two versions of that function, one that panics automatically (and that you're going to use 95% of the time) and another that returns a Result (e.g. for user-supplied regex). I get that argument, but imo that would be an even greater mess than having unwrap. It creates a burden for every library author to figure out how to handle errors, and to provide multiple options if necessary. The standard library goes this way sometimes, like with array access (foo[i] panics, foo.get(i) returns result) or println (println! panics, writeln! returns result). But the latter often catches people off guard because they don't expect it to panic. Propagating implicit panics to more places in the ecosystem in the name of getting rid of explicit panics with bad error messages (using unwrap) seems like a terrible tradeoff.
- eminence32 4y agoI am not convinced that propagation is massively superior to unwrap in example code. Unwraps are nice because they give you error messages with line numbers, but line numbers don't seem to come for free with propagation. Consider this code[1]: fn main() { let _my_num: u32 = "not a number".parse().unwrap(); } Running this gives a slightly noisy/verbose message, but to my eyes it's pretty useful: thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }', src/main.rs:2:47 Compare this to similar code with propagation[2]: fn main() -> Result<(), Box<dyn std::error::Error>> { let _my_num: u32 = "not a number".parse()?; Ok(()) } Not only do I find the extra boilerplate distracting for this example code, but if I wanted to run this code in a playground, the error message seems worse as it omits the line number: Error: ParseIntError { kind: InvalidDigit } It's true that crates like anyhow[3] have robust support for producing backtraces (and I use them all the time in non-example code). But I'm not inclined to add an external crate to my examples just to avoid calling an unwrap(). [1] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=20e2f21d6a296a09920525e6764837ff https://play.rust-lang.org/?version=stable&mode=debug&editio... [2] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9a7e8a1a7c0e81de81cc9b7e3c9ce04b https://play.rust-lang.org/?version=stable&mode=debug&editio... [3] https://crates.io/crates/anyhow https://crates.io/crates/anyhow
- dietrichepp 4y agoUgh. 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 agoI 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 agoI 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.
- happytoexplain 4y agoI'm not familiar with Rust, but it sounds like a similar story to `?`, `!`, `rethrows`, and `fatalError` in Swift. I.e.: - Handle nil cases with ? or propagate errors with rethrows. - If you really, seriously can't recover, use fatalError with a descriptive message. - ! is strictly for non-production code. It is just a hyper-short way of fatally failing with no message. Yes, maybe you can use it if you just checked for nil, but almost always you can refactor that to omit the !. Because of its convenience, people tend to abuse it.
- assbuttbuttass 4y agoUsing a mutex basically requires the use of unwrap let lockedValue = mutex.lock().unwrap(); The only time .lock() can fail is if another thread panicked and poisoned the mutex, so propagating the panic makes the most sense here.
- marcosdumay 4y agoLooks like an expect there would be an improvement. Do you write that line often enough that adding an error message is a problem?
- assbuttbuttass 4y agoThe reason I would hesitate to replace this with an expect is that I really don't ever expect .lock() to fail, so .expect just adds verbosity for no benefit. The article talks about the Regex::new method, which has much the same problem. For the majority of use cases, you don't expect it to ever fail. Maybe Mutex could introduce a new method .lock_or_die, because it's such a common pattern
- Matthias247 4y agoThere's not really a benefit to "except", since the stacktrace you get on the panic tells you exactly what happened (Mutex was poisoned). Repeating that in a custom error string doesn't make the behavior much better.
- nu11ptr 4y agoTLDR; - While I agree unwrap/expect should not typically be used often in production code there are enough exception cases that Rust would be MUCH worse if they didn't exist Longer version: Only skimmed, but I disagree, although I thought the same thing when I started. It sounds wonderful to have panic-free Rust but when when you sit down and think about how that would work you quickly realize it is a trade off between boilerplate and convenience. Rust code can be broken down into mostly three "types": 1. Safe, and can't panic (calls nothing that can panic) 2. Safe, can panic (.unwrap, .expect, indexing, etc.) 3. Unsafe, could do anything The goal when writing Rust IMO is to try and get as much as possible in #1, control #2 carefully by closely held invariants, and avoid #3 as much as possible (in many programs, this turns into no unsafe at all). The problem with avoiding #2 entirely is you will quickly get code that is hard to write, harder to understand, etc. Most often you will see situations like "I just proved this is x in length, so I can always index this with this value". If you don't do this you will have lots of error checking that effectively does nothing. I admit this happens rarely for Option/Result and in prod code I rarely use unwrap/expect, however, some code would be very tedious to write without unwrap or potentially panicking code, so unwrap/expect can be valuable for things like tests, examples, and rapid prototyping. I should probably add that I actually agree with the author that in the _typical_ case, you should not be using unwrap/expect, but either pattern matching it or propigating it with ?, but the exceptions to this (testing/examples/prototyping/corner cases) are important enough that I'm very glad Rust has the "escape hatch" of unwrap/expect.
- brundolf 4y agoI broadly disagree, but others here have covered all the points I would have made around that, so instead I'll mention an interesting thought this did bring up for me: > Most of the time I see it, ? would be better and could be used instead with minimal hassle I don't agree that you should just bring in a library, as a default strategy, to make it easy to use ? in all cases without thought But. For the subset of cases where you could do this without any type changes - where the unwrapped error already has the right type for the containing function - I wonder if it would make sense to have a Clippy rule and/or IDE autofix?
- topspin 4y agoMaybe it shouldn't have but developing in Rust would be intolerable without .unwrap(). In the real world you wade through API documentation figuring out the types and colors of functions while forever being assaulted by Result<>. Without .unwrap() your progress would be destroyed as you'd have to immediately deal with every unhappy path. unwrap() allows you nail together the happy path and then revisit the failure modes after you get your intentions working. Also, .unwrap() is fine for unit test mocking you need in real world code. Are irresponsible and/or under-resourced programmers abusing .unwrap()? Yes. Obviously. Fortunately, the frictionless .unwrap() makes this blatantly obvious to everyone else (just as profligate use of unsafe {} does,) so detecting crap code is much easier. This is a vast improvement over traditional 'systems' languages that permit said programmers to ignore failures without so much as a word. The net result is .unwrap() is a win for Rust.
- stormbrew 4y ago> Also, .unwrap() is fine for unit test mocking you need in real world code. People keep saying this but it is sometimes incredibly frustrating how little people annotate their test failure points in rust unit tests. I would really like to see more people get a little verbose in their tests and use the third arg of the assert macros and .expect with explanations of what the line is trying to prove.
- topspin 4y agoWhat I have in mind is when your unit test scaffold must populate some collection or whatnot from a file that's part of the code base, or similar legwork. The read might fail somehow but if it does you have bigger problems, so .unwrap() is fine.
- andrewaylett 4y agoPretty much all of my non-leaf functions return Result, in application code (or where I can't be bothered to build error types as I go) I'll use anyhow to allow me to coerce pretty much anything into my result type. So unwrap can -- fairly easily -- be replaced by `?`. I'd still like to be able to agree with the compiler that it can prove that certain error cases don't happen, as with ether an unthinking propagation upwards or an unwrap I'm reliant on my own analysis to assure myself I don't need to explicitly special-case that error. If the compiler is able to dead-code eliminate the unwrap, I'd like to be able to tell the compiler that there's a bug in my code if the unwrap is still in place after optimisation.
- codeflo 4y agoI agree unwrap is tragically overused in example code, and any non-logic error should use a Result instead — even early in a project. This isn’t a huge overhead. Even in “prototypey” code, it’s not hard to make everything return an anyhow::Result and upgrade to more fine-grained error types later. The main function can return a Result as well (which will be unwrapped by the runtime), making this style almost as concise as the unwrap version. I also agree that panic and crash is usually the correct response to a logic error. However, I think library functions like Regex::new shouldn’t decide for me what I do or don’t consider a logic error — how should the library know where that string comes from? They also shouldn’t be effectively required to offer two overloads for every function just to enable both decisions: there’s already a very clean way to put that decisions into the hands of the library user, Result.unwrap() vs. Result?. Now, about unwrap vs. expect: Since the author and I agree that panics should be reserved for logic errors, the expect() argument is effectively only a debugging aid. Should that be required? I’m torn. Often, which one I personally use reflects my confidence in getting the invariant correct and never hitting the panic in production. That can be risky. A particular team in a particular context might therefore adopt a convention to always use expect instead of unwrap, that’s fine. But I don’t think it’s unreasonable that the language offers a choice here, and in similar places, like the no-parameter variant of panic!().
- gwbas1c 4y agoTLDR: Industrial-strength code should not have "mystery error" failures. Always include some kind of error message; or adopt techniques to abstract away providing the error message.
- Animats 4y agoI tend to agree. You can use "unwrap()" on an Option, but you can't use "?". You have to write something like let val = v.ok_or(anyhow!("Error message goes here"))?; which is a bit unwieldy. fn square(v: &Option<f32>) -> f32 { Ok(v.unwrap()*v.unwrap()) } becomes fn square(v: &Option<f32>) -> Result<f32, Error> { let val = v.ok_or(anyhow!("None value sent to square"))?; Ok(val*val) } A default message would be useful. Bailing out of the middle of a map expression's closure is complicated. In that situation, "?" causes a return of the expression in the closure, not the whole map. There's a clever way around this.[1] It supposedly gets optimized so that the iteration quits early, and an array of Result types is not generated and then flattened. You never really need unwrap, but the "right way" can be verbose. [1] https://stackoverflow.com/questions/26368288/how-do-i-stop-iteration-and-return-an-error-when-iteratormap-returns-a-result https://stackoverflow.com/questions/26368288/how-do-i-stop-i...
- brink 4y agoYou can use a "?" on an option if the return type of the function is also option.
- stormbrew 4y agoFor the specific examples you have here you can shorten it a bit and use anyhow's OptionExt::context and avoid the closure. Someday maybe we'll have postfix macros and you'll even be able to format in it.
- throwaway234232 4y agoYour example doesn't help your argument here, as square should be taking `T` instead of `Option<T>`, a good use case for `Option::map`. But if you are speaking about ergonomics of handling Options, it is being improved upon with the usage of try operators with Options as well as possibly seeing try expressions in the near future. I don't think lack of ergonomics is a good justification for usage of unwrap, but that is entirely subjective. However, I do see the misappropriation of `unwrap` as a signal that the ergonomics in Rust are lacking and it's something that needs to be addressed, so again I do sympathize.
- marcosdumay 4y agoOk, that argument is about as old as Rust. With one difference that now the "?" operator exists. So, quick code that doesn't care about errors can be done quite simply by just using "?". The real place where there is a relevant use-case for unwrap is on code examples. And I am really not sure "?" is a good replacement for those, because there is a good chance that you will hit the people trying to learn from your example with a complicated type error before they even get some code to run.
- stuff4ben 4y ago[OT] Reading this article makes me want to use Rust so badly. I just don't have any places to use it. Everything at work is either Go or Scala or Java. Or front-end shit I'm not smart enough to do anymore. I want to find a job that'll pay me senior engineer rates but let me learn Rust like a baby and eventually do something useful with it...
- kif 4y ago> I want to find a job that'll pay me senior engineer rates but let me learn Rust like a baby and eventually do something useful with it You and me both. I've been learning Rust, slowly but surely. I'd like to use it at work, but most companies I've seen require experience with it.
- lordnacho 4y agoMy only issue with expect is it seems like the wrong name. It sounds like it means "I expect this condition to be true" whereas it really means "if this thing is not ok, print this message and panic". Why not just call it unwrap_or_errmsg?
- al2o3cr 4y agoI don't see how removing `unwrap` like the author dreams of would result in anything other than a lot of `expect("oh no")` instead - the problem refactors itself from "sloppy developers use unwrap which has a generic panic message" to "sloppy developers use expect with a generic panic message".
- cratermoon 4y agoSee also the discussion for a similar feature in Java, from "Optional: The Mother of all Bikesheds", where the use and misuse of get() has a central role. video: https://www.youtube.com/watch?v=Ej0sss6cq14 https://www.youtube.com/watch?v=Ej0sss6cq14 slides: https://stuartmarks.files.wordpress.com/2017/03/optionalmotherofallbikesheds-devoxxbe2016.pdf https://stuartmarks.files.wordpress.com/2017/03/optionalmoth...
- ghygghu 4y agoMy small gripe with unwrap is that the name is too similar to the perfectly safe unwrap_or (and friends). This is annoying both when reading the code and when grepping.
- GuB-42 4y agoIf a keyword is here, generally, it means there is a use for it. Feel free not to use it, but it is generally a better idea to try to understand why it is there before calling it a mistake. Here, it goes well with what is called "offensive programming", a take on "defensive programming". Here the idea is that you crash as soon as something unexpected happen. It is a bug, fix it instead of trying to recover from something you don't understand. It is actually common in embedded software, if something is wrong, stop execution and wait for the watchdog to trigger a restart.