[arithmetic_side_effects] Fix #11393

[c410-f3r]

Fix #11393

changelog: [`arithmetic_side_effects`]: Detect division by zero for `Wrapping` and `Saturating`

[rustbot]

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

[Manishearth]

@bors r+

thanks!

[bors]

📌 Commit d802ab2 has been approved by Manishearth

It is now in the queue for this repository.

[bors]

⌛ Testing commit d802ab2 with merge 706c48b...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 706c48b to master...

[guilliamxavier]

Sorry for being late, thanks for the fixes but I have several remarks/questions:

• The code and the tests in the two "arithmetic_side_effects.rs" files look like this intended to also fix [arithmetic_side_effects] Division by NonZeroU* cannot panic #11392?
• Yet "arithmetic_side_effects.stderr" looks like expecting the current, unfixed behavior (for both issues)? (probably related to the following points)
• The is_non_zero pattern is missing the *size variants
• Actually "If the RHS is NonZero*, then division or module by zero will never occur" (and the corresponding code) is too broad, only NonZeroU* (unsigned) are safe (actually the signed NonZeroI* don't even impl Div/Rem, but if they did then e.g. isize::MIN by NonZeroIsize(-1) would panic)
• "For `Saturation` or `Wrapping` (RHS), all but division and module are allowed" seems right, but "For `Saturation` or `Wrapping` (LHS), everything is allowed" is not: e.g. wrapping_x / wrapping_zero or wrapping_x /= primitive_zero (hopefully the naming is clear) will panic; maybe these two LHS and RHS checks should be combined into a single one?
• [suggestion] is_div_or_rem could be moved up before the first check and used in it too?

[c410-f3r]

This PR was meant to fix both #11392 and #11393 but the lack of the NonZeroUsize symbol and the performance cost of "interning" was a deal breaker in my perspective. I created a PR in the rust repository (rust-lang/rust#115177 (comment)) to improve this scenario and soon another PR using the newly introduced symbols will be submitted to finally close #11392.

You are right in regards to other topics and I will try to fix the concerns as soon as possible.