5 ms·

Say you're implementing quick-sort, and you're checking the pivot value like this: if (array.len() >= 2) { let pivot = array[array.len() / 2];

Say you're implementing quick-sort, and you're checking the pivot value like this: if (array.len() >= 2) { let pivot = array[array.len() / 2]; ... } Those brackets could panic, if the index was out of bounds! Of course you can tell locally from the code that it can't be out of bounds. But if you're really not allowed to have panics (and equivalently array accesses with square brackets, as those can panic), then you have to write this code instead: if (array.len() >= 2) { let pivot = match array.get(array.len() / 2) { Some(pivot) => pivot, None => return Err(InternalQuicksortError("bad index somehow")), }; ... } You do this all over your codebase and it gets hard to read very quickly! EDIT: clarified code

dangerlibrary 3y ago
Maybe my mind has been poisoned by a few years of Scala, but that just looks like normal code to me. Bundle up those errors and return a useful message to the user about why it failed, call it a day.
- defen 3y ago
  The point is that the language has decided that some kinds of builtin operations can panic, in the interest of developer ergonomics. The alternative is that every indexing operation returns a Result, or you write 4 lines of code instead of 1 for every indexing operation. So the notion that the programmer should never write "panic!" in their code does not fully address the underlying problem, if the reason for not writing "panic!" is "library code should never panic".
- justinpombrio 3y ago
  The point is that the code literally cannot panic under any circumstances. Just because there's a call to `unwrap()` or `[]` doesn't mean it's actually possible to trigger that panic. I made that more explicit the code, in case that wasn't clear. If there's any question as to whether a line might actually panic or not, then absolutely, generate helpful error messages. But when you can tell through simple local reasoning that the panic will never happen, you can just use square brackets. It's OK.
  - dangerlibrary 3y ago
    For the first time, I find myself wanting the equivalent of a :thumbs-up: reaction emoji on hn.
  - dllthomas 3y ago
    > when you can tell through simple local reasoning that the panic will never happen A problem is that you may be able to tell now, but as code changes and functions grow and split, that reasoning may grow less simple and less local without any changes of those lines in particular. The odds of that, and the degree to which it matters, are of course both spectacularly situation dependent.
    - fireflash38 3y ago
      And if my grandmother had wheels she'd be a bicycle! The point being, take context into account. You can't code like everything everywhere is going to get modified to be worse.
      - dllthomas 3y ago
        I agree, I was just surfacing a sometimes missed piece of context.
csnover 3y ago
Well all you have to do then is make it not hard to read :-) #[derive(thiserror::Error, Debug)] enum Error { #[error("bad index somehow")] BadIndex } // ... let pivot = array.get(array.len() / 2).ok_or(Error::BadIndex)?;
- morning-coffee 3y ago
  How does anyone on the stack above handle this error? i.e. what can be done programmatically to deal with it? I claim this is far worse than panic in this case... you've now introduced a code path variant that folks above think is "legitimate" or caused by some input or something in the environment that could be changed to make it go away... but that's not the case here... the only thing that can be fixed is the code in question to not be in this region if the array length is 2 or less. It's a plain old "bug" and the only cure (other than more cowbell) is to modify source and recompile.
  - phamilton 3y ago
    > How does anyone on the stack above handle this error? For a web application, you respond to the request with 5xx error. Applications do many things. A degraded state (e.g. a single endpoint being broken) is often preferable to a full crash. It's very little effort to opt in to panics by calling unwrap(). It's a lot of effort to recover from panics. Let me, as the caller, make that decision.
    - ramranch 3y ago
      But any standard web framework already would use a panic handler to turn a panic within application code to a 5xx error.
      - throwway3423 3y ago
        [flagged]
      - phamilton 3y ago
Joker_vD 3y ago
If only there was some way to propagate the range checks... let lo, hi = array.bounds() match lo < hi { None => array, Some(lo, hi) => { let pivot_index = lo.avg_with(hi) // guaranteed to be in bounds let pivot = array[pivot_index] // never panics, but the index type is opaque, not usize ...rest of the qsort here } }
- jedbrown 3y ago
  This is possible using branded indices. https://plv.mpi-sws.org/rustbelt/ghostcell/paper.pdf https://plv.mpi-sws.org/rustbelt/ghostcell/paper.pdf https://gitlab.mpi-sws.org/FP/ghostcell/-/blob/master/ghostcell/examples/branded_vec.rs#L96-97 https://gitlab.mpi-sws.org/FP/ghostcell/-/blob/master/ghostc...
  - Joker_vD 3y ago
    Yes, indeed. And it's not even a novel idea, my example is lifted straight from one of the references of the paper you've linked (namely, Kiselyov & Shan, "Lightweight Static Capabilities", 2007). I personally think that taking an optimization (e.g. boundary checks elimination) and bringing it — or more precisely, the logic that verifies that this optimization is safe — to the source language-level is a very promising direction of research.
- burntsushi 3y ago
  You're still using `array[index]` syntax though. That comes with a panicking branch. The optimizer might elide it, but it might just as well do that in the original code. I do not believe you've rebutted the main point here. At best what you've done is a show a better way of guaranteeing that indices are correct. But you still need to deal with the fact that index notation comes with a panicking branch.
  - Joker_vD 3y ago
    > That comes with a panicking branch Not necessarily? Indexing is overloadable, so nothing stops one to write an implementation that'd use an unsafe block without any checks/panics whatsoever (because there are static guarantees that those would be unnecessary).
lumb63 3y ago
I’d be curious to see if the optimizer couldn’t detect in this instance that that code is, in fact, unreachable. It seems to me like it should detect it with ease. I will concede that the “you should never get here” type errors are tempting to panic on. But I have seen so many instances where the “you should never get here” code does execute. Memory corruption, access by other processors, etc., could make this check (which should never happen!) actually fail at runtime. If one of those happens in code I wrote, I would rather be alerted to the error, without crashing, if possible. A lot of the appeal to “panic on error” IMO is to reduce developer cognitive load. As a developer, it’s inconvenient to consider all the errors that can actually happen when programming. Allocations, array accesses, I/O, and other errors programmers want to ignore, do happen. It’s annoying to think about them every time we write code, but I’d rather have something that forces me to consider them, rather than be bit by something that I designed out of my consideration. This preference might change depending on what I’m writing. For a one-off script that I need to spin up quickly, I don’t really care. For code running on a device I may not get to service for 5 years, my opinion would be different.
- zozbot234 3y ago
  > A lot of the appeal to “panic on error” IMO is to reduce developer cognitive load. As a developer, it’s inconvenient to consider all the errors that can actually happen when programming. Allocations, array accesses, I/O, and other errors programmers want to ignore, do happen. Rust has the ? operator to address the common pattern of returning a failure condition to the caller. There's effectively zero cognitive load involved, since compiler hints take care of it when writing the code. > I’d be curious to see if the optimizer couldn’t detect in this instance that that code is, in fact, unreachable. Probably, but that should be a job for the type checker not the optimizer. Ideally, there should be a way of creating statically verified asserts about program values, and plugging them in as preconditions to function calls that require them for correctness.
- justinpombrio 3y ago
  Wait hang on: your code hit a case that you know means something like memory corruption happened, and you want it to keep executing? If there's memory corruption, I want the program to stop ASAP so that it does as little damage as possible! FYI Rust panics and illegal `[]` accesses will tell you the file & line number where it happened.
tialaramex 3y ago
There are way too many replies here which seem like they fundamentally don't understand that you can't actually write an algorithm to solve the problem they imagine is easy. Rust does have Results that it knows can't fail - for genericity reasons - these are Infallible results, for example try_into() when into() would have worked is Infallible, Rust uses an empty type (equivalent to but for now not literally identical to ! aka Never) for this, so the compiler knows this type can't be reified, it needn't do code generation for this never-happens error. But there's a crucial distinction between Rust's compiler can see this is Infallible and I can show it won't fail and Rice's theorem means that gap is insurmountable even just in principle.
csomar 3y ago
You can convert the Option to a Result where the None becomes the error and then propagate from that. This is basically how you are using this Option by the way, and while it's possible, you have to consider why the original author decided to return an Option and not a Result: It's because he wants you to handle that in a way that doesn't fail (ie: an error) but instead consider it as a non-failure possibility.

dangerlibrary 3y ago

Maybe my mind has been poisoned by a few years of Scala, but that just looks like normal code to me. Bundle up those errors and return a useful message to the user about why it failed, call it a day.

defen 3y ago
The point is that the language has decided that some kinds of builtin operations can panic, in the interest of developer ergonomics. The alternative is that every indexing operation returns a Result, or you write 4 lines of code instead of 1 for every indexing operation. So the notion that the programmer should never write "panic!" in their code does not fully address the underlying problem, if the reason for not writing "panic!" is "library code should never panic".
justinpombrio 3y ago
The point is that the code literally cannot panic under any circumstances. Just because there's a call to `unwrap()` or `[]` doesn't mean it's actually possible to trigger that panic. I made that more explicit the code, in case that wasn't clear. If there's any question as to whether a line might actually panic or not, then absolutely, generate helpful error messages. But when you can tell through simple local reasoning that the panic will never happen, you can just use square brackets. It's OK.
- dangerlibrary 3y ago
  For the first time, I find myself wanting the equivalent of a :thumbs-up: reaction emoji on hn.
- dllthomas 3y ago
  > when you can tell through simple local reasoning that the panic will never happen A problem is that you may be able to tell now, but as code changes and functions grow and split, that reasoning may grow less simple and less local without any changes of those lines in particular. The odds of that, and the degree to which it matters, are of course both spectacularly situation dependent.
  - fireflash38 3y ago
    And if my grandmother had wheels she'd be a bicycle! The point being, take context into account. You can't code like everything everywhere is going to get modified to be worse.
    - dllthomas 3y ago
      I agree, I was just surfacing a sometimes missed piece of context.

csnover 3y ago

Well all you have to do then is make it not hard to read :-) #[derive(thiserror::Error, Debug)] enum Error { #[error("bad index somehow")] BadIndex } // ... let pivot = array.get(array.len() / 2).ok_or(Error::BadIndex)?;

morning-coffee 3y ago
How does anyone on the stack above handle this error? i.e. what can be done programmatically to deal with it? I claim this is far worse than panic in this case... you've now introduced a code path variant that folks above think is "legitimate" or caused by some input or something in the environment that could be changed to make it go away... but that's not the case here... the only thing that can be fixed is the code in question to not be in this region if the array length is 2 or less. It's a plain old "bug" and the only cure (other than more cowbell) is to modify source and recompile.
- phamilton 3y ago
  > How does anyone on the stack above handle this error? For a web application, you respond to the request with 5xx error. Applications do many things. A degraded state (e.g. a single endpoint being broken) is often preferable to a full crash. It's very little effort to opt in to panics by calling unwrap(). It's a lot of effort to recover from panics. Let me, as the caller, make that decision.
  - ramranch 3y ago
    But any standard web framework already would use a panic handler to turn a panic within application code to a 5xx error.
    - throwway3423 3y ago
      [flagged]
    - phamilton 3y ago

Joker_vD 3y ago

If only there was some way to propagate the range checks... let lo, hi = array.bounds() match lo < hi { None => array, Some(lo, hi) => { let pivot_index = lo.avg_with(hi) // guaranteed to be in bounds let pivot = array[pivot_index] // never panics, but the index type is opaque, not usize ...rest of the qsort here } }

jedbrown 3y ago
This is possible using branded indices. https://plv.mpi-sws.org/rustbelt/ghostcell/paper.pdf https://plv.mpi-sws.org/rustbelt/ghostcell/paper.pdf https://gitlab.mpi-sws.org/FP/ghostcell/-/blob/master/ghostcell/examples/branded_vec.rs#L96-97 https://gitlab.mpi-sws.org/FP/ghostcell/-/blob/master/ghostc...
- Joker_vD 3y ago
  Yes, indeed. And it's not even a novel idea, my example is lifted straight from one of the references of the paper you've linked (namely, Kiselyov & Shan, "Lightweight Static Capabilities", 2007). I personally think that taking an optimization (e.g. boundary checks elimination) and bringing it — or more precisely, the logic that verifies that this optimization is safe — to the source language-level is a very promising direction of research.
burntsushi 3y ago
You're still using `array[index]` syntax though. That comes with a panicking branch. The optimizer might elide it, but it might just as well do that in the original code. I do not believe you've rebutted the main point here. At best what you've done is a show a better way of guaranteeing that indices are correct. But you still need to deal with the fact that index notation comes with a panicking branch.
- Joker_vD 3y ago
  > That comes with a panicking branch Not necessarily? Indexing is overloadable, so nothing stops one to write an implementation that'd use an unsafe block without any checks/panics whatsoever (because there are static guarantees that those would be unnecessary).

lumb63 3y ago

I’d be curious to see if the optimizer couldn’t detect in this instance that that code is, in fact, unreachable. It seems to me like it should detect it with ease. I will concede that the “you should never get here” type errors are tempting to panic on. But I have seen so many instances where the “you should never get here” code does execute. Memory corruption, access by other processors, etc., could make this check (which should never happen!) actually fail at runtime. If one of those happens in code I wrote, I would rather be alerted to the error, without crashing, if possible. A lot of the appeal to “panic on error” IMO is to reduce developer cognitive load. As a developer, it’s inconvenient to consider all the errors that can actually happen when programming. Allocations, array accesses, I/O, and other errors programmers want to ignore, do happen. It’s annoying to think about them every time we write code, but I’d rather have something that forces me to consider them, rather than be bit by something that I designed out of my consideration. This preference might change depending on what I’m writing. For a one-off script that I need to spin up quickly, I don’t really care. For code running on a device I may not get to service for 5 years, my opinion would be different.

zozbot234 3y ago
> A lot of the appeal to “panic on error” IMO is to reduce developer cognitive load. As a developer, it’s inconvenient to consider all the errors that can actually happen when programming. Allocations, array accesses, I/O, and other errors programmers want to ignore, do happen. Rust has the ? operator to address the common pattern of returning a failure condition to the caller. There's effectively zero cognitive load involved, since compiler hints take care of it when writing the code. > I’d be curious to see if the optimizer couldn’t detect in this instance that that code is, in fact, unreachable. Probably, but that should be a job for the type checker not the optimizer. Ideally, there should be a way of creating statically verified asserts about program values, and plugging them in as preconditions to function calls that require them for correctness.
justinpombrio 3y ago
Wait hang on: your code hit a case that you know means something like memory corruption happened, and you want it to keep executing? If there's memory corruption, I want the program to stop ASAP so that it does as little damage as possible! FYI Rust panics and illegal `[]` accesses will tell you the file & line number where it happened.

tialaramex 3y ago

There are way too many replies here which seem like they fundamentally don't understand that you can't actually write an algorithm to solve the problem they imagine is easy. Rust does have Results that it knows can't fail - for genericity reasons - these are Infallible results, for example try_into() when into() would have worked is Infallible, Rust uses an empty type (equivalent to but for now not literally identical to ! aka Never) for this, so the compiler knows this type can't be reified, it needn't do code generation for this never-happens error. But there's a crucial distinction between Rust's compiler can see this is Infallible and I can show it won't fail and Rice's theorem means that gap is insurmountable even just in principle.

csomar 3y ago

You can convert the Option to a Result where the None becomes the error and then propagate from that. This is basically how you are using this Option by the way, and while it's possible, you have to consider why the original author decided to return an Option and not a Result: It's because he wants you to handle that in a way that doesn't fail (ie: an error) but instead consider it as a non-failure possibility.