Auto merge of #8226 - alexcrichton:lto-fixes, r=ehuss

Update assertions in LTO calculations

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

Author: bors

src/cargo/core/compiler/lto.rs

@@ -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),
@@ -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.

tests/testsuite/lto.rs

@@ -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();
+}