Add about overflow-checks flag in release mode #24
Conversation
There was a problem hiding this comment.
Thanks for adding some nuance @iglesias! Can you take a look at my comment?
| saturating, or wrapping operations. | ||
|
|
||
| .. rationale:: | ||
| :id: rat_kYiIiW8R2qD1 | ||
| :status: draft | ||
|
|
||
| In debug builds, Rust performs runtime checks for integer overflow and will panic if detected. | ||
| However, in release builds (with optimizations enabled), integer operations silently wrap | ||
| around on overflow, creating potential for silent failures and security vulnerabilities. | ||
| However, in release builds (with optimizations enabled), unless the flag overflow-checks is |
There was a problem hiding this comment.
While that will avoid overflow, I think that 'panic'ing may still be undesirable. I'd say if you added another sentence instead which described that silent wraps can be avoided with the overflow-checks flag, but we still panic, therefore we still advise being explicit that may work better. What do you think?
There was a problem hiding this comment.
Yeah, I think 'panic'inc is still undesirable in the context of this guideline's recommendation. I had considered whether it should be recommended adding it to the last sentence in the previous paragraph, then took a look at how the standard's library checked operation behaves (which is returning None) and decided not to. This change is about (probably a bit pedantically) note that the silent behavior isn't 100% implied by release / optimizations enabled. I will add another sentence here trying to clarify it.
iglesias
force-pushed
the
overflow-checks
branch
from
05fce40 to
bf131d2
Compare
|
Here is some additional context from the language team that I think might be useful: |
Hi, I thought this can add a bit of information. Flag documentation.