Skip to content
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

Stabilize -C overflow-checks #1535

Merged
merged 2 commits into from
Apr 21, 2016
Merged

Stabilize -C overflow-checks #1535

merged 2 commits into from
Apr 21, 2016

Conversation

brson
Copy link
Contributor

@brson brson commented Mar 9, 2016

Stabilize the -C overflow-checks command line argument.

This is an easy way to turn on overflow checks in release builds
without otherwise turning on debug assertions, via the -C debug-assertions flag. In stable Rust today you can't get one without
the other.

Rendered.

@@ -1,36 +0,0 @@
- Feature Name: (fill me in with a unique ident, my_awesome_feature)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably don't want to delete htis.

@alexcrichton alexcrichton added the T-dev-tools Relevant to the development tools team, which will review and decide on the RFC. label Mar 10, 2016
```

This may also be accomplished by Cargo's pending support for passing
arbitrary flags to rustc.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to wait for this, rather than add more ad hoc options to Cargo

@strega-nil
Copy link

This sounds amazing :D

@main--
Copy link

main-- commented Mar 15, 2016

I'd imagine that if the option were crate-specific, that would be an incentive for the crate authors to rely on specific overflow semantics, which is undesirable.

Is it? I'd say it's a perfectly reasonable choice for a crate to request overflow checks on all arithmetic operations except where explicitly disabled by using Wrapping<T>. I can for example see myself enabling this for a crate that deals heavily with unsafe code where integer overflow bugs could be disastrous.

@golddranks
Copy link

Ah, indeed, I was thinking it another way: if a crate has the option to NOT have overflow checks, then it might be a port to subtle undefined behaviour on different architectures.

Edit: To elaborate, the scenario I was thinking is: even if the checks are always on on debug, the crate author might be the only person to run the crate on debug, and not uncover some bugs. If he/she has the power to opt out of the overflow checks for the release version of his/her crate, then no users ever see that crate panicking on overflow, and they see the effects of overflows as device-specific functionality. Then, as an even rarer case, some user with a different architecture uses the crate, and once in a decade, his software is going to crash, and nobody ever knows why.

On the other hand, if the users are able to change the behaviour of the crate to panicking, then more people are likely to do so, and the bugs are found with a larger probability.

So, I wouldn't mind if there were a per-crate option: "always panicking" OR "either panics or silently overflows, decided by downstream", as long as there is no per-crate option "always silently overflow".

@golddranks
Copy link

Actually, now that I think of it, it would be pretty nice if you could build with overflow-checks on or off, but an individual crate or function could opt from "downstream decides" to "always panic".

Just a confirmation: as it currently stands in the RFC text, the flag -C overflow-checks=off doesn't preclude the compiler from doing per-crate or per-function overflow checks, if such functionality were later added, does it?

@brson
Copy link
Contributor Author

brson commented Apr 5, 2016

Just a confirmation: as it currently stands in the RFC text, the flag -C overflow-checks=off doesn't preclude the compiler from doing per-crate or per-function overflow checks, if such functionality were later added, does it?

I think that's right. If we had explicit overflow annotations in code they would take precedence.

@alexcrichton
Copy link
Member

🔔 This RFC is now entering its week-long final comment period 🔔

@alexcrichton alexcrichton added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Apr 5, 2016
@alexcrichton
Copy link
Member

The tools team discussed this RFC during triage the other day and the conclusion was to merge. Thanks again for the discussion!

@alexcrichton
Copy link
Member

Tracking issue: rust-lang/rust#33134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-arithmetic Arithmetic related proposals & ideas A-machine Proposals relating to Rust's abstract machine. final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-dev-tools Relevant to the development tools team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants