Document panicking cases for integer division and remainder

[jturner314]

This PR documents the cases when integer division and remainder operations panic. These operations panic in two cases: division by zero and overflow.

It's surprising that these operations always panic on overflow, unlike most other arithmetic operations, which panic on overflow only when debug_assertions is enabled. The panic on overflow for the remainder is also surprising because a return value of 0 would be reasonable in this case. ("Overflow" occurs only for MIN % -1.) Since the panics on overflow are somewhat surprising, they should be documented.

I guess it's worth asking: is panic on overflow (even when debug_assertions is disabled) the intended behavior? If not, what's the best way forward?

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @KodrAus (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[nikomatsakis]

RFC 560 states:

The operations /, % for the arguments INT_MIN and -1 will unconditionally panic. This is unconditional for legacy reasons.

Exactly what legacy reasons I had in mind I'm not sure, that sentence kind of amuses me now. =) Nonetheless, I'd say it is clearly intentional, and I imagine that any change here would have to be done over an edition.

[ojeda]

Those operations' behavior can differ greatly between architectures (e.g. hardware exception vs. returning zero for integer division by zero), so perhaps you intended to normalize it to make things more portable/defined than e.g. C?

[jturner314]

RFC 560 states:

The operations /, % for the arguments INT_MIN and -1 will unconditionally panic. This is unconditional for legacy reasons.

Okay, thanks; I missed that statement in the RFC. Since the existing behavior is intentional, I don't propose changing it. I'd just like to describe the existing behavior in the API docs.

Those operations' behavior can differ greatly between architectures (e.g. hardware exception vs. returning zero for integer division by zero), so perhaps you intended to normalize it to make things more portable/defined than e.g. C?

Woah, do you mean that in Rust, integer division by zero may return zero instead of panicking, or do you mean that this is a platform-specific internal implementation detail hidden by the language?

If Rust does not guarantee that division by zero panics, we should definitely document the set of possible behaviors for division by zero. I've been operating under the assumption that it always results in a panic, and I think I've written code that depends on the panicking behavior for correctness/soundness, which I now need to go and fix...

Would the following documentation be correct for division for signed integers?

This operation rounds towards zero, truncating any fractional part of the exact result.

Division by Zero

Division by zero either panics or returns an incorrect result, such as 0 or MAX.

Overflow

Division unconditionally panics on overflow (i.e. MIN / -1), regardless of whether overflow-checks or debug-assertions is enabled.

[ojeda]

Woah, do you mean that in Rust, integer division by zero may return zero instead of panicking, or do you mean that this is a platform-specific internal implementation detail hidden by the language?

The latter (i.e. I was replying to @nikomatsakis). For instance, under x86_64 a divide error is raised for both, which means in Linux you will get sent SIGFPE and your program terminated unless handled. However, under AArch64 you will get 0 (for division by zero) or INT_MIN (for INT_MIN / -1); and your program will go on its merry way. In C both situations are undefined likely for this reason.

Rust currently emits three branches to check for this and panic if needed, so it seems it is working as expected. I agree it would be nice to document this. :)

[jturner314]

Thanks for the clarification. I'm glad that the language guarantees panicking on integer division by zero. The proposed documentation in this PR is correct, then. As I see it, the remaining things to discuss before merging are code style (of the macro changes) and any desired adjustments to the wording in the docs (such as explicitly mentioning overflow-checks/debug-assertions or the fact that MIN / -1 is the only case where overflow occurs).

[JohnTitor]

r? @nikomatsakis as per I-nominated + T-lang

[nikomatsakis]

@bors r+

[bors]

📌 Commit b458550 has been approved by nikomatsakis