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.

Sign up for GitHub

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

Jump to bottom

Stop -O/-C opt-level and -g/-C debuginfo conflicting #60426

Merged
merged 7 commits into from
May 6, 2019
Merged

Stop -O/-C opt-level and -g/-C debuginfo conflicting #60426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6da57f
Fix argument alias handling for -O and -C opt-level
varkor Apr 30, 2019
e8e43c9
Fix argument alias handling for -g and -C debuginfo
varkor Apr 30, 2019
11b1d48
Add a run-make test for command aliases
varkor Apr 30, 2019
d54cf00
Add comments
varkor May 1, 2019
ecd0468
Update getopts to 0.2.19
varkor May 2, 2019
12f63ec
Move run-make test to run-make-fulldeps
varkor May 4, 2019
80f54ba
Update Cargo.lock
varkor May 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ dependencies = [
"cc 1.0.35 (registry+https://github.com/rust-lang/crates.io-index)",
"cmake 0.1.38 (registry+https://github.com/rust-lang/crates.io-index)",
"filetime 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)",
"num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -463,7 +463,7 @@ dependencies = [
"diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
"env_logger 0.5.13 (registry+https://github.com/rust-lang/crates.io-index)",
"filetime 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
Expand All @@ -484,7 +484,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)",
"filetime 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"miow 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -974,7 +974,7 @@ dependencies = [

[[package]]
name = "getopts"
version = "0.2.18"
version = "0.2.19"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -1989,7 +1989,7 @@ version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"bitflags 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
Expand All @@ -2008,7 +2008,7 @@ version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"memchr 2.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"unicase 2.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
]
Expand Down Expand Up @@ -3095,7 +3095,7 @@ dependencies = [
"dirs 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
"env_logger 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
"failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"ignore 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -3522,7 +3522,7 @@ dependencies = [
name = "test"
version = "0.0.0"
dependencies = [
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"proc_macro 0.0.0",
"term 0.0.0",
]
Expand All @@ -3532,7 +3532,7 @@ name = "tester"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)",
"term 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
]
Expand Down Expand Up @@ -4132,7 +4132,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum futures 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)" = "1a70b146671de62ec8c8ed572219ca5d594d9b06c0b364d5e67b722fc559b48c"
"checksum fwdansi 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "34dd4c507af68d37ffef962063dfa1944ce0dd4d5b82043dbab1dabe088610c3"
"checksum generic-array 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ef25c5683767570c2bbd7deba372926a55eaae9982d7726ee2a1050239d45b9d"
"checksum getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)" = "0a7292d30132fb5424b354f5dc02512a86e4c516fe544bb7a25e7f266951b797"
"checksum getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)" = "72327b15c228bfe31f1390f93dd5e9279587f0463836393c9df719ce62a3e450"
"checksum git2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c7339329bfa14a00223244311560d11f8f489b453fb90092af97f267a6090ab0"
"checksum git2-curl 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d58551e903ed7e2d6fe3a2f3c7efa3a784ec29b19d0fbb035aaf0497c183fbdd"
"checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ build_helper = { path = "../build_helper" }
cmake = "0.1.38"
filetime = "0.2"
num_cpus = "1.0"
getopts = "0.2.18"
getopts = "0.2.19"
cc = "1.0.35"
libc = "0.2"
serde = "1.0.8"
Expand Down
31 changes: 25 additions & 6 deletions src/librustc/session/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2181,10 +2181,21 @@ pub fn build_session_options_and_crate_config(
TargetTriple::from_triple(host_triple())
};
let opt_level = {
if matches.opt_present("O") {
if cg.opt_level.is_some() {
early_error(error_format, "-O and -C opt-level both provided");
// The `-O` and `-C opt-level` flags specify the same setting, so we want to be able
// to use them interchangeably. However, because they're technically different flags,
// we need to work out manually which should take precedence if both are supplied (i.e.
// the rightmost flag). We do this by finding the (rightmost) position of both flags and
// comparing them. Note that if a flag is not found, its position will be `None`, which
// always compared less than `Some(_)`.
let max_o = matches.opt_positions("O").into_iter().max();
varkor marked this conversation as resolved.
Show resolved Hide resolved
let max_c = matches.opt_strs_pos("C").into_iter().flat_map(|(i, s)| {
if let Some("opt-level") = s.splitn(2, '=').next() {
Some(i)
} else {
None
}
}).max();
if max_o > max_c {
OptLevel::Default
} else {
match cg.opt_level.as_ref().map(String::as_ref) {
Expand All @@ -2208,11 +2219,19 @@ pub fn build_session_options_and_crate_config(
}
}
};
// The `-g` and `-C debuginfo` flags specify the same setting, so we want to be able
// to use them interchangeably. See the note above (regarding `-O` and `-C opt-level`)
// for more details.
let debug_assertions = cg.debug_assertions.unwrap_or(opt_level == OptLevel::No);
let debuginfo = if matches.opt_present("g") {
if cg.debuginfo.is_some() {
early_error(error_format, "-g and -C debuginfo both provided");
let max_g = matches.opt_positions("g").into_iter().max();
let max_c = matches.opt_strs_pos("C").into_iter().flat_map(|(i, s)| {
if let Some("debuginfo") = s.splitn(2, '=').next() {
Some(i)
} else {
None
}
}).max();
let debuginfo = if max_g > max_c {
DebugInfo::Full
} else {
match cg.debuginfo {
Expand Down
2 changes: 1 addition & 1 deletion src/libtest/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ path = "lib.rs"
crate-type = ["dylib", "rlib"]

[dependencies]
getopts = "0.2.18"
getopts = "0.2.19"
term = { path = "../libterm" }

# not actually used but needed to always have proc_macro in the sysroot
Expand Down
22 changes: 22 additions & 0 deletions src/test/run-make-fulldeps/override-aliased-flags/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
-include ../tools.mk

# FIXME: it would be good to check that it's actually the rightmost flags
# that are used when multiple flags are specified, but I can't think of a
# reliable way to check this.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we make rustc show or warn to stderr about which flag it chose? "Note: both foo and bar specified. Using bar", then, we could just capture its output and compare.

Copy link
Contributor

Choose a reason for hiding this comment

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

(cc @varkor for his comment on #60426 (comment) which I read just now)


all:
# Test that `-O` and `-C opt-level` can be specified multiple times.
# The rightmost flag will be used over any previous flags.
$(RUSTC) -O -O main.rs
$(RUSTC) -O -C opt-level=0 main.rs
$(RUSTC) -C opt-level=0 -O main.rs
$(RUSTC) -C opt-level=0 -C opt-level=2 main.rs
$(RUSTC) -C opt-level=2 -C opt-level=0 main.rs

# Test that `-g` and `-C debuginfo` can be specified multiple times.
# The rightmost flag will be used over any previous flags.
$(RUSTC) -g -g main.rs
$(RUSTC) -g -C debuginfo=0 main.rs
$(RUSTC) -C debuginfo=0 -g main.rs
$(RUSTC) -C debuginfo=0 -C debuginfo=2 main.rs
$(RUSTC) -C debuginfo=2 -C debuginfo=0 main.rs
1 change: 1 addition & 0 deletions src/test/run-make-fulldeps/override-aliased-flags/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}