Is it worth putting an attribute on `unwrap()` family of functions?

[mkpankov]

Disclaimer: please forgive my ignorance if I miss something. I’m not experienced with Rust yet.

Today, it’s common to find code like

extern crate term;

fn main() {
    let mut t = term::stdout().unwrap();

    t.fg(term::color::GREEN).unwrap();
    (write!(t, "hello, ")).unwrap();

    t.fg(term::color::RED).unwrap();
    (writeln!(t, "world!")).unwrap();

    t.reset().unwrap();
}

As one might learn pretty quickly, unwrap()ing a None returned by term::stdout() will lead to failure of task. This is, of course, specified in docs. But it’s certainly not obvious that this is not a “safe” operation — it may lead to “crash”. Not “hard” crash, because it will be handled by Rust runtime. It produces error about unwrapping None. But it’s a crash nevertheless. And it happens due to several reasons:

1. Absence of warnings from compiler that .unwrap() is essentially a pattern match w/o “catch-all” case. We do get warnings when not all variants of enum are handled, so why skip it here?
2. Usage in official documentation — in example code like that of term. Conditions programmer to accept it as something normal.

Let’s face it — unwrap() of Option<_> is awful hack. It’s convenient, I know — but it’s the sort of convenience we disdain JavaScript and PHP for. I also understand that one is extremely likely to use unwrap() in closures because “heck, I only need to filter/map/reduce stuff, I don’t want my closure to spend 5 lines!”. Programmer uses it there and then forgets about it. Voila — safety is needlessly violated.

I think a programmer should be forced to actively discard any failure condition. It makes for explicit decision making about not handling an error.

In Rust, in it’s present state, unwrap() is the easiest and simplest way to Get Things Done. This shortcut will be taken by many and many people who will then later complain about Rust not being secure despite claiming itself “preventing almost all crashes”.

I have at least two ideas about it:

1. Introduce attribute, like #[unsafe] or something, indicating that the function will panic!() on some inputs. The compiler would then issue warnings (or errors) about usage of such functions. And it probably can be extended to everything that can call panic!() inside (although I’m very much not sure about this last point. It might turn out we need to warn about 99% of standard library).
2. Promote safer error handling in documentation more. All the lazy unwrap()s can be replaced with proper idiomatic error handling code.

I’m eager to hear opinion of more experienced rustaceans.

[sinistersnare]

almost every function can panic. println! can panic, so if you are trying to printf-debug something, then you would need to add your #[unsafe] everywhere.

armin ronacher’s blog has a lot of good writing on this, especially this post

I disagree with your assessment that unwrap is an “awful hack,” it is nowhere near a hack. You use unwrap if you want your program to crash on None, and it is used in documentation because documentation is for the API, not for teaching error handling basics.

I also think its terrible to think that it is as bad as JS or PHP, you need to explicitly handle the failure in Rust, in some cases, it is through unwrap

Using pattern matching based error handling over unwrap will produce just as many crashes over unwrap calls, because that is what people want.

I recommend you work more with the language until you make ideas on how to fix it. It ain’t broke.

[nodakai]

I think it’s fair to frown upon too many casual uses of unwrap(). My understanding is it implies an assertion as strong as assert!.

You use unwrap if you want your program to crash on None

I wonder how much % of all the usages of unwrap() is really meant to "crash on None". There are definitely quite a few of them which were written a part of “boilerplate” without thinking what they really implicate. At least for Option, expect() with a descriptive error message should be preffered over unwrap().

[mkpankov]

println! can panic, so if you are trying to printf-debug something

What is not a surprise, considering C’s printf() does memory allocation inside and println() ought to do roughly the same stuff. And you might remember it’s far-reaching controversy of C world — that nobody handles potential failure of printing. Of course, it doesn’t happen most of the time and who cares, but then somewhere in the embedded it does… I had to care about it because of printing stuff where allocating could mess things up, like during ELF program segments mapping. So we’ve got another chance to properly fix another C blight and I think we shouldn’t run from it. Look at unwrap() from perspective of student or a person not familiar with that borrowed-from-Haskell monadic error handling stuff. When you see unwrap(), it’s purely superficial — it does nothing useful, it doesn’t convey information. We could strip it down and automatically coerce Option<..> to Some(_) in case user doesn’t care to pattern match it. But that would be, of course, unfortunate. So I think we should go the opposite direction. Speaking of Haskell — when first writing the post yesterday, I wanted to argue Haskell doesn’t have such a function. I learned Haskell for ~2 years. And I didn’t know fromJust exists. Because nobody — LYAH, RWH, or any random paper or tutorial on the internet — uses it. The point being — like hell we should stop showing newbies unwrap() because “we’re only illustrating an API”. It kind of defeats the purpose.

you need to explicitly handle the failure in Rust

Problem is, unwrap() isn’t explicit enough.

Using pattern matching based error handling over unwrap will produce just as many crashes over unwrap calls, because that is what people want.

Making it inconvenient (or even painful) to ignore error handling will actually make people not want ignoring it. Then they will stop for a moment while writing proper match maybe_term {..} and think “hm, well, I wonder under which conditions we might fail to acquire a terminal?..” And that’s a good thing.

I recommend you work more with the language until you make ideas on how to fix it. It ain’t broke.

Is it not broken or are these ideas of fixing it not good enough?

[nodakai]

By the way, why don’t we join this ongoing discussion on a novel convention for error handling…?

Rust is evolving so quickly that I can hardly keep up with it, but I hope I can make time to read mitsuhiko’s latest proposal this weekend.

• http://www.reddit.com/r/rust/comments/2n6mpm/feedback_wanted_improved_rust_error_handling/

and

http://discuss.rust-lang.org/t/idea-for-improved-error-handling-ergonomics/904

[arielb1]

Haskell hides its errors in exceptions, so it doesn’t need to unwrap so often. If you use task isolation properly (e.g. you are a script, or the worker task of a GUI), then unwrap is the best way to handle impossible-to-proceed errors.

[glaebhoerl]

I think these functions should be renamed to assert() to more forthrightly convey the connotations. Relevant RFC. This is somewhat controversial, with plenty of people taking the opposite view, so I doubt it will happen.

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.