Auto merge of rust-lang#8657 - ehuss:fix-doctest-lto, r=alexcrichton

Fix LTO with doctests.

This fixes an issue where `cargo test --release` would fail to run doctests if LTO is set in `profile.release` or `profile.bench`.

The issue is that dependencies were built with `-Clinker-plugin-lto`, but the final rustdoc invocation did not issue `-C lto`. This causes a link failure (or crash!) because the necessary object code was missing.  This is because rustdoc historically did not support codegen flags, so Cargo has never passed them in.  Rustdoc now supports codegen flags (via rust-lang/rust#63827), so it should be safe to start passing them in.  For now, I am only adding LTO, but more should be added in the future.

There are two bugs here. One is that LTO flags aren't passed to rustdoc. The other is that the "doctest" unit was using the wrong profile (it was using dev/release when it should be using test/bench).

There are two distinct scenarios here.  One where just `release` has `lto` set.  And one where both `release` and `bench` have `lto` set.  This is relevant because LTO mostly cares about what the final artifact wants, and in the case where `bench` does not have `lto` set, then LTO will not be used for tests. This will hopefully be a little cleaner in the future when rust-lang#6988 is stabilized, which causes the test/bench profiles to *inherit* from the dev/bench profiles, which means you won't need to manually synchronize the test/bench profiles with dev/release.

Fixes rust-lang#8654

src/cargo/core/compiler/context/mod.rs

@@ -210,7 +210,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             // Collect information for `rustdoc --test`.
             if unit.mode.is_doc_test() {
                 let mut unstable_opts = false;
-                let args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
+                let mut args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
+                args.extend(compiler::lto_args(&self, unit));
                 self.compilation.to_doc_test.push(compilation::Doctest {
                     unit: unit.clone(),
                     args,

src/cargo/core/compiler/mod.rs

@@ -797,28 +797,9 @@ fn build_base_args(
         cmd.arg("-C").arg(format!("panic={}", panic));
     }
 
-    match cx.lto[unit] {
-        lto::Lto::Run(None) => {
-            cmd.arg("-C").arg("lto");
-        }
-        lto::Lto::Run(Some(s)) => {
-            cmd.arg("-C").arg(format!("lto={}", s));
-        }
-        lto::Lto::Off => {
-            cmd.arg("-C").arg("lto=off");
-        }
-        lto::Lto::ObjectAndBitcode => {} // this is rustc's default
-        lto::Lto::OnlyBitcode => {
-            cmd.arg("-C").arg("linker-plugin-lto");
-        }
-        lto::Lto::OnlyObject => {
-            cmd.arg("-C").arg("embed-bitcode=no");
-        }
-    }
+    cmd.args(&lto_args(cx, unit));
 
     if let Some(n) = codegen_units {
-        // There are some restrictions with LTO and codegen-units, so we
-        // only add codegen units when LTO is not used.
         cmd.arg("-C").arg(&format!("codegen-units={}", n));
     }
 
@@ -946,6 +927,23 @@ fn build_base_args(
     Ok(())
 }
 
+fn lto_args(cx: &Context<'_, '_>, unit: &Unit) -> Vec<OsString> {
+    let mut result = Vec::new();
+    let mut push = |arg: &str| {
+        result.push(OsString::from("-C"));
+        result.push(OsString::from(arg));
+    };
+    match cx.lto[unit] {
+        lto::Lto::Run(None) => push("lto"),
+        lto::Lto::Run(Some(s)) => push(&format!("lto={}", s)),
+        lto::Lto::Off => push("lto=off"),
+        lto::Lto::ObjectAndBitcode => {} // this is rustc's default
+        lto::Lto::OnlyBitcode => push("linker-plugin-lto"),
+        lto::Lto::OnlyObject => push("embed-bitcode=no"),
+    }
+    result
+}
+
 fn build_deps_args(
     cmd: &mut ProcessBuilder,
     cx: &mut Context<'_, '_>,

src/cargo/core/profiles.rs

@@ -299,7 +299,7 @@ impl Profiles {
             let release = matches!(self.requested_profile.as_str(), "release" | "bench");
 
             match mode {
-                CompileMode::Test | CompileMode::Bench => {
+                CompileMode::Test | CompileMode::Bench | CompileMode::Doctest => {
                     if release {
                         (
                             InternedString::new("bench"),
@@ -312,10 +312,7 @@ impl Profiles {
                         )
                     }
                 }
-                CompileMode::Build
-                | CompileMode::Check { .. }
-                | CompileMode::Doctest
-                | CompileMode::RunCustomBuild => {
+                CompileMode::Build | CompileMode::Check { .. } | CompileMode::RunCustomBuild => {
                     // Note: `RunCustomBuild` doesn't normally use this code path.
                     // `build_unit_profiles` normally ensures that it selects the
                     // ancestor's profile. However, `cargo clean -p` can hit this

tests/testsuite/lto.rs

@@ -1,6 +1,6 @@
 use cargo::core::compiler::Lto;
 use cargo_test_support::registry::Package;
-use cargo_test_support::{project, Project};
+use cargo_test_support::{basic_manifest, project, Project};
 use std::process::Output;
 
 #[cargo_test]
@@ -514,16 +514,19 @@ fn cdylib_and_rlib() {
     p.cargo("test --release -v --manifest-path bar/Cargo.toml")
         .with_stderr_unordered(
             "\
-[FRESH] registry v0.0.1
-[FRESH] registry-shared v0.0.1
+[COMPILING] registry v0.0.1
+[COMPILING] registry-shared v0.0.1
+[RUNNING] `rustc --crate-name registry [..]-C embed-bitcode=no[..]
+[RUNNING] `rustc --crate-name registry_shared [..]-C embed-bitcode=no[..]
 [COMPILING] bar [..]
+[RUNNING] `rustc --crate-name bar [..]--crate-type cdylib --crate-type rlib [..]-C embed-bitcode=no[..]
 [RUNNING] `rustc --crate-name bar [..]-C embed-bitcode=no --test[..]
 [RUNNING] `rustc --crate-name b [..]-C embed-bitcode=no --test[..]
 [FINISHED] [..]
-[RUNNING] [..]
-[RUNNING] [..]
+[RUNNING] [..]target/release/deps/bar-[..]
+[RUNNING] [..]target/release/deps/b-[..]
 [DOCTEST] bar
-[RUNNING] `rustdoc --crate-type cdylib --crate-type rlib --test [..]
+[RUNNING] `rustdoc --crate-type cdylib --crate-type rlib --test [..]-C embed-bitcode=no[..]
 ",
         )
         .run();
@@ -627,7 +630,7 @@ fn test_profile() {
 [COMPILING] bar v0.0.1
 [RUNNING] `rustc --crate-name bar [..]crate-type lib[..]
 [COMPILING] foo [..]
-[RUNNING] `rustc --crate-name foo [..]--crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no[..]
+[RUNNING] `rustc --crate-name foo [..]--crate-type lib --emit=dep-info,metadata,link -C linker-plugin-lto[..]
 [RUNNING] `rustc --crate-name foo [..]--emit=dep-info,link -C lto=thin [..]--test[..]
 [FINISHED] [..]
 [RUNNING] [..]
@@ -680,7 +683,7 @@ fn dev_profile() {
 [COMPILING] bar v0.0.1
 [RUNNING] `rustc --crate-name bar [..]crate-type lib[..]
 [COMPILING] foo [..]
-[RUNNING] `rustc --crate-name foo [..]--crate-type lib --emit=dep-info,metadata,link -C linker-plugin-lto [..]
+[RUNNING] `rustc --crate-name foo [..]--crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no [..]
 [RUNNING] `rustc --crate-name foo [..]--emit=dep-info,link -C embed-bitcode=no [..]--test[..]
 [FINISHED] [..]
 [RUNNING] [..]
@@ -689,3 +692,57 @@ fn dev_profile() {
 ")
         .run();
 }
+
+#[cargo_test]
+fn doctest() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+                edition = "2018"
+
+                [profile.release]
+                lto = true
+
+                [dependencies]
+                bar = { path = "bar" }
+            "#,
+        )
+        .file(
+            "src/lib.rs",
+            r#"
+                /// Foo!
+                ///
+                /// ```
+                /// foo::foo();
+                /// ```
+                pub fn foo() { bar::bar(); }
+            "#,
+        )
+        .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
+        .file(
+            "bar/src/lib.rs",
+            r#"
+                pub fn bar() { println!("hi!"); }
+            "#,
+        )
+        .build();
+
+    p.cargo("test --doc --release -v")
+        .with_stderr_contains("[..]`rustc --crate-name bar[..]-C embed-bitcode=no[..]")
+        .with_stderr_contains("[..]`rustc --crate-name foo[..]-C embed-bitcode=no[..]")
+        // embed-bitcode should be harmless here
+        .with_stderr_contains("[..]`rustdoc [..]-C embed-bitcode=no[..]")
+        .run();
+
+    // Try with bench profile.
+    p.cargo("test --doc --release -v")
+        .env("CARGO_PROFILE_BENCH_LTO", "true")
+        .with_stderr_contains("[..]`rustc --crate-name bar[..]-C linker-plugin-lto[..]")
+        .with_stderr_contains("[..]`rustc --crate-name foo[..]-C linker-plugin-lto[..]")
+        .with_stderr_contains("[..]`rustdoc [..]-C lto[..]")
+        .run();
+}