Consider panicking in ArrayVec::remove instead of returning an Option

[tbu-]

A version bump would make this change possible: Vec panics on out-of-bounds indices: Vec::remove.

[tbu-]

Also in insert. Currently out of bounds cannot be distinguished from a capacity error. IMO it should panic on the first, and return an error for the latter.

[bluss]

I am very skeptical of an API that both panics and returns Result.

[tbu-]

Why? They're different classes of errors – see e.g. the from_str_radix function of integers in the standard library: If you pass a base not within the range from 2 to 36, it panics, because it is a programmer error. I see a parallel here to how indices are handled in Vec: Invalid indices panic, they are considered a programmer error. from_str_radix returns a Err if the string does not parse to a number – like the other ArrayVec methods that return an error on capacity overflow.

[bluss]

Thanks for the example. (Bug rust-lang/rust/issues/42034 is about what to do with that. Not having it documented is of course the worst of both worlds 😄)

Returning a Result signals that the function uses Results for its error handing; so it's simpler and more straightforward if it has no other error cases than the ones visible in the type system.

I understand the contract violation argument, that it's a whole different class of error, but it does make a more complicated rule for the user to understand. So as a guideline / first start, I prefer either wholly panic or wholly Result for error handling in a single method.