3 ms·

> Crucially, by grepping for `unwrap` and `expect` you can find the panics very easily. I write Rust code up and down the stack, and this is the thing that tic

> Crucially, by grepping for `unwrap` and `expect` you can find the panics very easily. I write Rust code up and down the stack, and this is the thing that ticks me off more than anything else. You can get most of them looking for this stuff (and a handful of others), but there are just so many places in vanilla Rust that exhibit the Wrong Behavior By Default, e.g. a[i] = x + 1; Which looks so innocent but can panic in two places! Ugh.

throwaway894345 4y ago
One can conceive of tooling that can find these panics for you so you don't need to grep, but I'm not aware of anything that's been written at this time.
pitaj 4y ago
There are two clippy lints that cover these possible panics: - indexing_slicing - integer_arithmetic Enabling those will throw an error on usage in your codebase. Also it's worth noting that arithmetic is only checked against overflow when building in debug mode by default.
- couchand 4y ago
  > Also it's worth noting that arithmetic is only checked against overflow when building in debug mode by default. Yes, the default behavior in release mode is even worse!
sophacles 4y ago
Sure, you wrote 2 overflows - a buffer one and an integer one. Would you rather it silently breaks?
- couchand 4y ago
  No, I'd rather it not compile!
tialaramex 4y ago
If x's Add doesn't have panic on overflow then you get rid of the panic risk on the right hand side. For example if x is Wrapping<u8> then it can't panic. Having chosen Wrapping<u8> you will presumably not be startled when 255 + 1 == 0 Or if x is Saturating<i16> again, no panic, but 32767 + 1 == 32767 Wrapping and Saturating really exist (well Saturating isn't stable, but it's in nightly) and if you're in a space where that's what you want, you can express it cleanly in Rust. Most programmers just mean the integers they learned in school which don't have a "maximum" or "minimum" they go on forever, and so overflow means the programmer's model of the world mapping those integers to a type like usize was wrong and this code is faulty. [Edited to add: This doesn't quite work because in the end neither Wrapping nor Saturating implement Add over the normal integers, since it's not certain what you intend if you wrote that, better to have an error asking you to explain. AddAssign works though] Now, there aren't any built in types whose IndexMut can't panic AFAIK, but we could build one, and if a was such a type then a[i] doesn't panic either. For example we can imagine a is a (hypothetical) PythonDict while x is a Saturating<i16> and now our code just compiles to add 1 to x, saturating if necessary, and then shove the result in our PythonDict under the value of i.
- couchand 4y ago
  Yes, Wrapping and Saturating very neatly provide behavior you might want in some context. I use them a lot in embedded contexts. Since indexing is nearly always fallible, it should be fallible!
  - tialaramex 4y ago
    Rust's slices (the most obvious thing you would index) have three ways to index them, if you are out of bounds the consequences are Undefined Behaviour; panic; and return None. Today those are provided by get_unchecked(), the Index operator and get() respectively. [[ Calling get_unchecked() is unsafe of course, but it's potentially faster because it has no bounds check ]] I'm confident we sometimes want each of the three options, so if we're having the Index operator return None then get() presumably panics, or at least some new function is provided with that behaviour. I reckon this just drives annoyance that [k] has to be unwrapped, while get(k) does what people actually wanted [k] to do. I could be wrong, but that's my sense.

throwaway894345 4y ago

One can conceive of tooling that can find these panics for you so you don't need to grep, but I'm not aware of anything that's been written at this time.

pitaj 4y ago

There are two clippy lints that cover these possible panics: - indexing_slicing - integer_arithmetic Enabling those will throw an error on usage in your codebase. Also it's worth noting that arithmetic is only checked against overflow when building in debug mode by default.

couchand 4y ago
> Also it's worth noting that arithmetic is only checked against overflow when building in debug mode by default. Yes, the default behavior in release mode is even worse!

sophacles 4y ago

Sure, you wrote 2 overflows - a buffer one and an integer one. Would you rather it silently breaks?

couchand 4y ago
No, I'd rather it not compile!

tialaramex 4y ago

If x's Add doesn't have panic on overflow then you get rid of the panic risk on the right hand side. For example if x is Wrapping<u8> then it can't panic. Having chosen Wrapping<u8> you will presumably not be startled when 255 + 1 == 0 Or if x is Saturating<i16> again, no panic, but 32767 + 1 == 32767 Wrapping and Saturating really exist (well Saturating isn't stable, but it's in nightly) and if you're in a space where that's what you want, you can express it cleanly in Rust. Most programmers just mean the integers they learned in school which don't have a "maximum" or "minimum" they go on forever, and so overflow means the programmer's model of the world mapping those integers to a type like usize was wrong and this code is faulty. [Edited to add: This doesn't quite work because in the end neither Wrapping nor Saturating implement Add over the normal integers, since it's not certain what you intend if you wrote that, better to have an error asking you to explain. AddAssign works though] Now, there aren't any built in types whose IndexMut can't panic AFAIK, but we could build one, and if a was such a type then a[i] doesn't panic either. For example we can imagine a is a (hypothetical) PythonDict while x is a Saturating<i16> and now our code just compiles to add 1 to x, saturating if necessary, and then shove the result in our PythonDict under the value of i.

couchand 4y ago
Yes, Wrapping and Saturating very neatly provide behavior you might want in some context. I use them a lot in embedded contexts. Since indexing is nearly always fallible, it should be fallible!
- tialaramex 4y ago
  Rust's slices (the most obvious thing you would index) have three ways to index them, if you are out of bounds the consequences are Undefined Behaviour; panic; and return None. Today those are provided by get_unchecked(), the Index operator and get() respectively. [[ Calling get_unchecked() is unsafe of course, but it's potentially faster because it has no bounds check ]] I'm confident we sometimes want each of the three options, so if we're having the Index operator return None then get() presumably panics, or at least some new function is provided with that behaviour. I reckon this just drives annoyance that [k] has to be unwrapped, while get(k) does what people actually wanted [k] to do. I could be wrong, but that's my sense.