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

Can not override previous opt-levels in list of command flags #7493

Closed
mstewartgallus opened this issue Jun 30, 2013 · 19 comments · Fixed by #60426
Closed

Can not override previous opt-levels in list of command flags #7493

mstewartgallus opened this issue Jun 30, 2013 · 19 comments · Fixed by #60426
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@mstewartgallus
Copy link
Contributor

If one passes multiple opt-level flags to the compiler (for example -O, and --opt-level=3) then an error results. Instead, the previous setting should be overridden. This is bad because then makefiles don't work easily. For example, I had to use the command line make RUSTFLAGS=--opt-level=3 DISABLE_OPTIMIZE=1 instead of the more obvious make RUSTFLAGS=--opt-level=3 when compiling rust as optimized because of this error.

@catamorphism
Copy link
Contributor

Yeah, this should be fixed. Maybe in addition, there should be a warning message saying something like "you supplied -O and --opt-level=3, but I'm assuming you meant --opt-level=3".

@flaper87
Copy link
Contributor

visiting for triage. This is still valid and should be easy to fix. Setting tags respectively and myself as mentor.

EDIT: The error raised is -O and --opt-level both provided and it is raised by rustc::driver::driver::build_session_options

@thestinger
Copy link
Contributor

I'm not really convinced that this should be changed. A Makefile can set a variable to a default if and only if it's not set manually, and then include it in the command-line.

@flaper87
Copy link
Contributor

@thestinger that's also a good solution. I'm fine with both, TBH. @alexcrichton any opinion on this? Otherwise I think we can close this issue for now.

@huonw
Copy link
Member

huonw commented Apr 16, 2014

gcc, ghc, ldmd2 and clang all seem to accept multiple --opt-level/-O params and just take the last one (just the few compilers I had immediately to hand).

@flaper87
Copy link
Contributor

@huonw I find that behavior to be more intuitive but I don't have a strong opinion for this specific case.

@alexcrichton
Copy link
Member

I could go either way on this. While not exactly kosher, it is convenient to have overrides like this sometimes.

@kl4ng
Copy link

kl4ng commented Apr 25, 2014

Hey, I'm looking to fix this bug.

Should the default behavior be to accept "--opt-level=X" over "-O" if both are given?
Or should it accept whichever appears last?

if the latter, after looking at the getopts module, it doesn't seem as if there is a way to determine the ordering among two arguments, unless I'm mistaken.

@mstewartgallus
Copy link
Contributor Author

Whichever is last should be overridden @kl4ng so that appended options can override each other.
It should be possible to have default options that are then overridden as later options are passed in.

@jgallagher
Copy link

Should this issue be closed?

@mahkoh
Copy link
Contributor

mahkoh commented Oct 11, 2014

This bug should be fixed, either by fixing getopts or by moving rustc to another argument parser.

@jgallagher
Copy link

Was it not fixed by the closed PR?

@mahkoh
Copy link
Contributor

mahkoh commented Oct 11, 2014

The PR was closed and not merged.

@huonw
Copy link
Member

huonw commented Oct 11, 2014

@jgallagher it was closed, not merged. :)

@jgallagher
Copy link

Ahh my mistake, it was closed without merging.

@tamird
Copy link
Contributor

tamird commented Apr 18, 2015

This discussion should probably move to the RFcs repo. cc @steveklabnik

@steveklabnik steveklabnik added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Apr 18, 2015
@brson
Copy link
Contributor

brson commented Jun 27, 2016

Visiting for triage. Specifying -O twice works, specifying -C opt-level twice works. Mixing them does not. I think it should be fixed. This is easy to do. There is explicit code in rustc::session::config that turns this scenario into an error. Instead it should figure out which one appeared last in the argument list and choose it. May require a bit of a hack to make that happen, scanning env::args independently of the getopts processing. Add a test case to src/run-make.

@brson brson removed the A-build label Jun 27, 2016
@sanxiyn
Copy link
Member

sanxiyn commented Jul 6, 2016

I did this in #32399. The conclusion then was this needs changes to getopts.

@brson brson removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jul 8, 2016
@brson
Copy link
Contributor

brson commented Jul 8, 2016

Removing easy label because lack of agreement on way forward.

@Mark-Simulacrum Mark-Simulacrum added the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Jun 23, 2017
@Mark-Simulacrum Mark-Simulacrum removed the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jun 23, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 19, 2017
Centril added a commit to Centril/rust that referenced this issue May 3, 2019
…r=alexcrichton

Stop `-O`/`-C opt-level` and `-g`/`-C debuginfo` conflicting

Allow `-O` and `-C opt-level`, and `-g` and `-C debuginfo` to be specified simultaneously without conflict. In such cases, the rightmost provided option is chosen.

Fixes rust-lang#7493.
Fixes rust-lang#32352.

~Blocked on rust-lang/getopts#79

r? @alexcrichton
bors added a commit that referenced this issue May 4, 2019
Stop `-O`/`-C opt-level` and `-g`/`-C debuginfo` conflicting

Allow `-O` and `-C opt-level`, and `-g` and `-C debuginfo` to be specified simultaneously without conflict. In such cases, the rightmost provided option is chosen.

Fixes #7493.
Fixes #32352.

~Blocked on rust-lang/getopts#79

r? @alexcrichton
Manishearth added a commit to Manishearth/rust that referenced this issue May 5, 2019
…r=alexcrichton

Stop `-O`/`-C opt-level` and `-g`/`-C debuginfo` conflicting

Allow `-O` and `-C opt-level`, and `-g` and `-C debuginfo` to be specified simultaneously without conflict. In such cases, the rightmost provided option is chosen.

Fixes rust-lang#7493.
Fixes rust-lang#32352.

~Blocked on rust-lang/getopts#79

r? @alexcrichton
flip1995 pushed a commit to flip1995/rust that referenced this issue Jul 29, 2021
…impl-type, r=camsteffen

Prefer a code snipped over formatting the self type (`new_without_default`)

Fixes: rust-lang/rust-clippy#7220

changelog: [`new_without_default`]: The `Default` impl block type doesn't use the full type path qualification

Have a nice day to everyone reading this 🙃
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet