Skip to content

Commit

Permalink
Update assertions in LTO calculations
Browse files Browse the repository at this point in the history
Turns out a case I thought didn't happen can indeed happen. Units may
depend on other units which are LTO-able because integration tests can
depend on binaries. Handle that case internally and remove a few panics.

Closes #8223
  • Loading branch information
alexcrichton committed May 8, 2020
1 parent 4399670 commit 2a97225
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 8 deletions.
17 changes: 9 additions & 8 deletions src/cargo/core/compiler/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ fn calculate(
(Lto::None, false)
} else if unit.target.can_lto() {
// Otherwise if this target can perform LTO then we're going to read the
// LTO value out of the profile.
assert!(!require_bitcode); // can't depend on binaries/staticlib/etc
// LTO value out of the profile. Note that we ignore `require_bitcode`
// here because if a unit depends on another unit than can LTO this
// isn't a rustc-level dependency but rather a Cargo-level dependency.
// For example this is an integration test depending on a binary.
match unit.profile.lto {
profiles::Lto::Named(s) => match s.as_str() {
"n" | "no" | "off" => (Lto::Run(Some(s)), false),
Expand All @@ -73,12 +75,11 @@ fn calculate(

Entry::Occupied(mut v) => {
let result = match (lto, v.get()) {
// Targets which execute LTO cannot be depended on, so these
// units should only show up once in the dependency graph, so we
// should never hit this case.
(Lto::Run(_), _) | (_, Lto::Run(_)) => {
unreachable!("lto-able targets shouldn't show up twice")
}
// Once we're running LTO we keep running LTO. We should always
// calculate the same thing here each iteration because if we
// see this twice then it means, for example, two unit tests
// depend on a binary, which is normal.
(Lto::Run(s), _) | (_, &Lto::Run(s)) => Lto::Run(s),

// If we calculated the same thing as before then we can bail
// out quickly.
Expand Down
58 changes: 58 additions & 0 deletions tests/testsuite/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,61 @@ fn between_builds() {
p.cargo("build -v --release --lib").run();
p.cargo("build -v --release").run();
}

#[cargo_test]
fn test_all() {
if !cargo_test_support::is_nightly() {
return;
}

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
[profile.release]
lto = true
"#,
)
.file("src/main.rs", "fn main() {}")
.file("tests/a.rs", "")
.file("tests/b.rs", "")
.build();
p.cargo("test --release -v")
.with_stderr_contains("[RUNNING] `rustc[..]--crate-name foo[..]-C lto[..]")
.run();
}

#[cargo_test]
fn test_all_and_bench() {
if !cargo_test_support::is_nightly() {
return;
}

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
[profile.release]
lto = true
[profile.bench]
lto = true
"#,
)
.file("src/main.rs", "fn main() {}")
.file("tests/a.rs", "")
.file("tests/b.rs", "")
.build();
p.cargo("test --release -v")
.with_stderr_contains("[RUNNING] `rustc[..]--crate-name a[..]-C lto[..]")
.with_stderr_contains("[RUNNING] `rustc[..]--crate-name b[..]-C lto[..]")
.with_stderr_contains("[RUNNING] `rustc[..]--crate-name foo[..]-C lto[..]")
.run();
}

0 comments on commit 2a97225

Please sign in to comment.