-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Style: do not defensively use saturating_sub()
#13513
Conversation
r? @Manishearth rustbot has assigned @Manishearth. Use |
Sgtm 👍 @bors r+ |
Style: do not defensively use `saturating_sub()` Using `saturating_sub()` here in code which cannot fail brings a false sense of security. If for any reason a logic error was introduced and caused `self.loop_depth` to reach 0 before being decremented, using `saturating_sub(1)` would silently mask the programming error instead of panicking loudly as it should (at least in dev profile). changelog: none
Using `saturating_sub()` here in code which cannot fail brings a false sense of security. If for any reason a logic error was introduced and caused `self.loop_depth` to reach 0 before being decremented, using `saturating_sub(1)` would silently mask the programming error instead of panicking loudly as it should (at least in dev profile).
7018975
to
af6816c
Compare
Fun fact, I force-pushed (with only a new commit message, content unchanged) after you r+'d it. I guess bors will keep the previous one, which is fine. |
The force push might have confused bors 🤔 |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Using
saturating_sub()
here in code which cannot fail brings a false sense of security. If for any reason a logic error was introduced and causedself.loop_depth
to reach 0 before being decremented, usingsaturating_sub(1)
would silently mask the programming error instead of panicking loudly as it should (at least in dev profile).changelog: none