You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After that we'd just need to add a test which validates this behavior, likely a src/test/run-pass/*.rs addition with a // compile-flags: ... annotation at the top.
Likewise the Cargo support should be relatively easy:
OK, so this is pretty straightforward. Couple questions after actually implementing this:
The RFC says the option is -C overflow-checks, but comments on the RFC suggest that people expect this to be -C overflow-checks(=on/off), so one could have (I guess) -C debug-assertions -C overflow-checks=off. Which variant should be supported?
It seems better to make the value in CodegenOptions the single source of truth to be used throughout the compiler by setting it based on the value of -C debug-assertions or -Z force-overflow-checks; this makes the uses easier to read and also makes any future removal of -Z force-overflow-checks straightforward. How should -C overflow-checks interact with -Z force-overflow-checks? Should we just consider any explicit setting of -C overflow-checks to "win" and only consult -Z force-overflow-checks if the -C option was not explicitly provided? I guess that would mean that we'd almost have to implement the -C overflow-checks(=on/off) variant above?
@froydnj I think it's fine to just reuse the existing parse_opt_bool which accepts a "standard variety" of boolean options.
It seems better to make the value in CodegenOptions the single source of truth
Makes sense to me to have a single source of truth! Right now I don't think we have a lot of precedent for modifying CodegenOptions after creation so this may not be easy. Perhaps a helper method on Session though? That's what a few other codegen-like options do.
How should -C overflow-checks interact with -Z force-overflow-checks?
Yeah let's just have -C trump -Z. The precise semantics likely don't matter too much anyway as we'll quickly phase out the -Z version anyway.
Activity
alexcrichton commentedon Feb 16, 2017
FWIW this should be a relatively easy bug to knock out!
The
force_overflow_checksoption is defined with other debugging options and we'd just want to add a new flag to the associated codegen options (we'll delete the-Zflag once everyone's migrated).There's a few uses throughout the codebase which I believe will need an update:
svh_visitor.rshair/cx/mod.rstrans/base.rsAfter that we'd just need to add a test which validates this behavior, likely a
src/test/run-pass/*.rsaddition with a// compile-flags: ...annotation at the top.Likewise the Cargo support should be relatively easy:
parse itbuild_base_argswith the new optionfroydnj commentedon Feb 16, 2017
I'd like to tackle this!
froydnj commentedon Feb 17, 2017
OK, so this is pretty straightforward. Couple questions after actually implementing this:
-C overflow-checks, but comments on the RFC suggest that people expect this to be-C overflow-checks(=on/off), so one could have (I guess)-C debug-assertions -C overflow-checks=off. Which variant should be supported?CodegenOptionsthe single source of truth to be used throughout the compiler by setting it based on the value of-C debug-assertionsor-Z force-overflow-checks; this makes the uses easier to read and also makes any future removal of-Z force-overflow-checksstraightforward. How should-C overflow-checksinteract with-Z force-overflow-checks? Should we just consider any explicit setting of-C overflow-checksto "win" and only consult-Z force-overflow-checksif the-Coption was not explicitly provided? I guess that would mean that we'd almost have to implement the-C overflow-checks(=on/off)variant above?alexcrichton commentedon Feb 17, 2017
@froydnj I think it's fine to just reuse the existing
parse_opt_boolwhich accepts a "standard variety" of boolean options.Makes sense to me to have a single source of truth! Right now I don't think we have a lot of precedent for modifying
CodegenOptionsafter creation so this may not be easy. Perhaps a helper method onSessionthough? That's what a few other codegen-like options do.Yeah let's just have
-Ctrump-Z. The precise semantics likely don't matter too much anyway as we'll quickly phase out the-Zversion anyway.add `-C overflow-checks` option
Rollup merge of rust-lang#40037 - froydnj:overflow-checks, r=alexcric…
Rollup merge of rust-lang#40037 - froydnj:overflow-checks, r=alexcric…
Remove `-Z force-overflow-checks`
Rollup merge of rust-lang#91185 - camelid:rm-force-overflow-checks, r…
Rollup merge of rust-lang#91185 - camelid:rm-force-overflow-checks, r…
Rollup merge of rust-lang#91185 - camelid:rm-force-overflow-checks, r…