3 ms·

There's plenty of situations where telling developers to go change their code so it doesn't panic might be an appropriate lint. Think safety critical rust for e

There's plenty of situations where telling developers to go change their code so it doesn't panic might be an appropriate lint. Think safety critical rust for example.

sshine 2y ago
Adding to this argument: A lot of panics are placed instead of proper error handling as a cause of happy-path programming. (E.g. all cases of .unwrap()) Some panics are placed not knowing they’re panics. (E.g. indexing, calling a function that panics) Some panics are legitimately placed by people who want a program to panic at a given place; this may collide with the caller’s desire to use Result and error types. There are lots of situations where being notified about where the panics live can make your code more robust, because they do sneak in.
- SkiFire13 2y ago
  But what about those panics for "I know this situation cannot possibly happen"?
  - sshine 2y ago
    For those you have `unreachable!()` (a hope rather than a certainty): https://doc.rust-lang.org/std/macro.unreachable.html https://doc.rust-lang.org/std/macro.unreachable.html In those cases you strike a deal with the devil. The type system won't help you. Maybe it's an acceptable tradeoff for readability and efficiency. Maybe you could have modelled the code so it wasn't necessary. I think you can divide the use of `unreachable!()` in two cases: 1) As a consequence of style (applying LEM to code) 2) As a consequence of low-level abstractions One can be avoided by remodelling. The other can be tucked away behind safe interfaces.
    - steveklabnik 2y ago
      The unreachable macro invokes a panic. It wouldn’t be allowed if panics were not allowed.
    - Fulgen 2y ago
      As the other commenter said, `unreachable!()` panics. You'd need https://doc.rust-lang.org/core/hint/fn.unreachable_unchecked.html https://doc.rust-lang.org/core/hint/fn.unreachable_unchecked...
      - SkiFire13 2y ago
        Which is arguably even worse, because now if it fails you don't even see it! You wanted to make the code more robust and ultimately made it less.
        sshine 2y ago
        Yes, I'm not saying `unreachable!()` isn't a `panic!()`. And I am also not arguing for the use of `unreachable_unchecked!()`. I am only arguing that in all the other cases mentioned, you can simply avoid panics.
        SkiFire13 2y ago
        The problem however is that a generic "panic" lint will trigger on these as well. Which will be pretty annoying. Combined with the fact that such a lint will have to see through functions in other crates (otherwise `unwrap` and indexing won't be linted) will make this pretty annoying. I hope maintainers won't be nagged to remove lecit panics like these just because the lint annoys some dependents...
duped 2y ago
My claim is that's not actually very useful for ensuring the correctness of code, since Rust doesn't dependent typing.
- AlotOfReading 2y ago
  Correctness of safety critical code (and others) is about more than just memory and logical safety. Imagine your brake control module is running rust and something unwinds. Now your time budget is being spent unwinding the stack and you're probably going to run into a watchdog halfway through, forcing a reboot and possibly escalating into a hard stop depending on the safety model. You can imagine similar issues with cryptography for example, or network stacks. I'm not saying that lint should be enabled by default. It's a highly specialized config, but it's an important protection when it's useful.

There's plenty of situations where telling developers to go change their code so it doesn't panic might be an appropriate lint. Think safety critical rust for example.

sshine 2y ago

Adding to this argument: A lot of panics are placed instead of proper error handling as a cause of happy-path programming. (E.g. all cases of .unwrap()) Some panics are placed not knowing they’re panics. (E.g. indexing, calling a function that panics) Some panics are legitimately placed by people who want a program to panic at a given place; this may collide with the caller’s desire to use Result and error types. There are lots of situations where being notified about where the panics live can make your code more robust, because they do sneak in.

SkiFire13 2y ago
But what about those panics for "I know this situation cannot possibly happen"?
- sshine 2y ago
  For those you have `unreachable!()` (a hope rather than a certainty): https://doc.rust-lang.org/std/macro.unreachable.html https://doc.rust-lang.org/std/macro.unreachable.html In those cases you strike a deal with the devil. The type system won't help you. Maybe it's an acceptable tradeoff for readability and efficiency. Maybe you could have modelled the code so it wasn't necessary. I think you can divide the use of `unreachable!()` in two cases: 1) As a consequence of style (applying LEM to code) 2) As a consequence of low-level abstractions One can be avoided by remodelling. The other can be tucked away behind safe interfaces.
  - steveklabnik 2y ago
    The unreachable macro invokes a panic. It wouldn’t be allowed if panics were not allowed.
  - Fulgen 2y ago
    As the other commenter said, `unreachable!()` panics. You'd need https://doc.rust-lang.org/core/hint/fn.unreachable_unchecked.html https://doc.rust-lang.org/core/hint/fn.unreachable_unchecked...
    - SkiFire13 2y ago
      Which is arguably even worse, because now if it fails you don't even see it! You wanted to make the code more robust and ultimately made it less.
      - sshine 2y ago
        Yes, I'm not saying `unreachable!()` isn't a `panic!()`. And I am also not arguing for the use of `unreachable_unchecked!()`. I am only arguing that in all the other cases mentioned, you can simply avoid panics.
        SkiFire13 2y ago
        The problem however is that a generic "panic" lint will trigger on these as well. Which will be pretty annoying. Combined with the fact that such a lint will have to see through functions in other crates (otherwise `unwrap` and indexing won't be linted) will make this pretty annoying. I hope maintainers won't be nagged to remove lecit panics like these just because the lint annoys some dependents...

duped 2y ago

My claim is that's not actually very useful for ensuring the correctness of code, since Rust doesn't dependent typing.

AlotOfReading 2y ago
Correctness of safety critical code (and others) is about more than just memory and logical safety. Imagine your brake control module is running rust and something unwinds. Now your time budget is being spent unwinding the stack and you're probably going to run into a watchdog halfway through, forcing a reboot and possibly escalating into a hard stop depending on the safety model. You can imagine similar issues with cryptography for example, or network stacks. I'm not saying that lint should be enabled by default. It's a highly specialized config, but it's an important protection when it's useful.