It seems people have a blind spot for unwrap, perhaps because it's so often used in example code. In production code an unwrap or expect should be reviewed exactly like a panic.

It's not necessarily invalid to use unwrap in production code if you would just call panic anyway. But just like every unsafe block needs a SAFETY comment, every unwrap in production code needs an INFALLIBILITY comment. clippy::unwrap_used can enforce this.

▲

dehrmann an hour ago | parent | next [-]

It's the same blind spot people have to Java's checked exceptions. People commonly resort to Pokemon exception handling and either blindly ignoring or rethrowing as a runtime exception. When Rust got popular, I was a bit confused by people talking about how great Result it's essentially a checked exception without a stack trace.

	▲	gwbas1c 27 minutes ago \| parent \| next [-]
		It's a lot lighter: a stack trace takes a lot of overhead to generate; a result has no overhead for a failure. The overhead (panic) only comes once the failure can't be handled. (Most books on Java/C# don't explain that throwing exceptions has high performance overhead.) Exceptions force a panic on all errors, which is why they're supposed to be used in "exceptional" situations. To avoid exceptions when an error is expected, (eof, broken socket, file not found,) you either have to use an unnatural return type or accept the performance penalty of the panic that happens when you "throw." In Rust, the stack trace happens at panic (unwrap), which is when the error isn't handled. IE, it's not when the file isn't found, it's when the error isn't handled.
	▲	Terr_ 40 minutes ago \| parent \| prev [-]
		"Checked Exceptions Are Actually Good" gang, rise up! :p I think adoption would have played out very different if there had only been some more syntactic-sugar. For example, an easy syntax for saying: "In this method, any (checked) DeepException e that bubbles up should trigger a new (checked) MyException, with e as the linked cause." We might still get lazy programmers making systems where everything throws a vague MyException, but even a statically-checkable mess would be way easier to fix later.

▲

dist1ll 2 hours ago | parent | prev | next [-]

> every unwrap in production code needs an INFALLIBILITY comment. clippy::unwrap_used can enforce this.

How about indexing into a slice/map/vec? Should every `foo[i]` have an infallibility comment? Because they're essentially `get(i).unwrap()`.

▲

10000truths 2 hours ago | parent | next [-]

Yes? Funnily enough, I don't often use indexed access in Rust. Either I'm looping over elements of a data structure (in which case I use iterators), or I'm using an untrusted index value (in which case I explicitly handle the error case). In the rare case where I'm using an index value that I can guarantee is never invalid (e.g. graph traversal where the indices are never exposed outside the scope of the traversal), then I create a safe wrapper around the unsafe access and document the invariant.

▲

dist1ll an hour ago | parent [-]

If that's the case then hats off. What you're describing is definitely not what I've seen in practice. In fact, I don't think I've ever seen a crate or production codebase that documents infallibility of every single slice access. Even security-critical cryptography crates that passed audits don't do that. Personally, I found it quite hard to avoid indexing for graph-heavy code, so I'm always on the lookout for interesting ways to enforce access safety. If you have some code to share that would be very interesting.

▲

hansvm 39 minutes ago | parent [-]

> graph-heavy code

Could you share some more details, maybe one fully concrete scenario? There are lots of techniques, but there's no one-size-fits-all solution.

	▲	dist1ll 20 minutes ago \| parent [-]
		Sure, these days I'm mostly working on a few compilers. Let's say I want to make a fixed-size SSA IR. Each instruction has an opcode and two operands (which are essentially pointers to other instructions). The IR is populated in one phase, and then lowered in the next. During lowering I run a few peephole and code motion optimizations on the IR, and then do regalloc + asm codegen. During that pass the IR is mutated and indices are invalidated/updated. The important thing is that this phase is extremely performance-critical.

▲

tux3 2 hours ago | parent | prev | next [-]

Usually you'd want to write almost all your slice or other container iterations with iterators, in a functional style.

For the 5% of cases that are too complex for standard iterators? I never bother justifying why my indexes are correct, but I don't see why not.

You very rarely need SAFETY comments in Rust because almost all the code you write is safe in the first place. The language also gives you the tool to avoid manual iteration (not just for safety, but because it lets the compiler eliminate bounds checks), so it would actually be quite viable to write these comments, since you only need them when you're doing something unusual.

▲

wrs an hour ago | parent | next [-]

I didn't restate the context from the code we're discussing: it must not panic. If you don't care if the code panics, then go ahead and unwrap/expect/index, because that works with chosen error handling scheme. This is fine for lots of things like CLI tools or isolated subprocesses, and makes review a lot easier.

So: first, identify code that cannot be allowed to panic. Within that code, yes, in the rare case that you use [i], you need to at least try to justify why you think it'll be in bounds. But it would be better not to.

There are a couple of attempts at getting the compiler to prove that code can't panic (e.g., the no-panic crate).

	▲	kibwen an hour ago \| parent [-]
		Indexing is comparatively rare given the existence of iterators, IMO. If your goal is to avoid any potential for panicking, I think you'd have a harder time with arithmetic overflow.

▲

dist1ll 2 hours ago | parent | prev [-]

For iteration, yes. But there's other cases, like any time you have to deal with lots of linked data structures. If you need high performance, chances are that you'll have to use an index+arena strategy. They're also common in mathematical codebases.

▲

danielheath 2 hours ago | parent | prev [-]

I mean... yeah, in general. That's what iterators are for.

▲

speed_spread an hour ago | parent | prev [-]

Pet peeve: unwrap() should be deprecated and renamed or_panic(). More consistent with the rest of stdlib methods and appropriately scarier.

	▲	echelon a minute ago \| parent [-]
		A lot of stuff should be done about the awful unwrap family of methods. A few ideas: - It should not compile in production Rust code - It should only be usable within unsafe blocks - It should require explicit "safe" annotation from the engineer. Though this is subject to drift and become erroneous. - It should be possible to ban the use of unsafe in dependencies and transitive dependencies within Cargo.