Auto merge of #8658 - ehuss:beta-fix-doctest-lto, r=alexcrichton

[beta] Fix LTO with doctests. Beta packports: * #8653 Fix cache_messages::rustdoc test broken on beta. — so that CI tests pass * #8648 Add spaces after -C and -Z flags for consistency — so that #8657 can be merged * #8657 Fix LTO with doctests. Although the LTO regression is on 1.46, I probably won't do a stable backport even if there is a point release, as I think it would be good to have sufficient time for testing on nightly, and I don't want to rush it. I'm willing to reconsider that, though.
rust-lang · Aug 28, 2020 · f3c7e06 · f3c7e06
2 parents 51b6612 + d1ff577
commit f3c7e06
Show file tree

Hide file tree

Showing 5 changed files with 124 additions and 72 deletions.
diff --git a/src/cargo/core/compiler/context/mod.rs b/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,

diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs
@@ -207,7 +207,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
 
     rustc.args(cx.bcx.rustflags_args(unit));
     if cx.bcx.config.cli_unstable().binary_dep_depinfo {
-        rustc.arg("-Zbinary-dep-depinfo");
+        rustc.arg("-Z").arg("binary-dep-depinfo");
     }
     let mut output_options = OutputOptions::new(cx, unit);
     let package_id = unit.pkg.package_id();
@@ -533,7 +533,7 @@ fn prepare_rustc(
     if cx.bcx.config.cli_unstable().jobserver_per_rustc {
         let client = cx.new_jobserver()?;
         base.inherit_jobserver(&client);
-        base.arg("-Zjobserver-token-requests");
+        base.arg("-Z").arg("jobserver-token-requests");
         assert!(cx.rustc_clients.insert(unit.clone(), client).is_none());
     } else {
         base.inherit_jobserver(&cx.jobserver);
@@ -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("-Clinker-plugin-lto");
-        }
-        lto::Lto::OnlyObject => {
-            cmd.arg("-Cembed-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));
     }
 
@@ -862,7 +843,7 @@ fn build_base_args(
         // will simply not be needed when the behavior is stabilized in the Rust
         // compiler itself.
         if *panic == PanicStrategy::Abort {
-            cmd.arg("-Zpanic-abort-tests");
+            cmd.arg("-Z").arg("panic-abort-tests");
         }
     } else if test {
         cmd.arg("--cfg").arg("test");
@@ -922,7 +903,8 @@ fn build_base_args(
         // any non-public crate in the sysroot).
         //
         // RUSTC_BOOTSTRAP allows unstable features on stable.
-        cmd.arg("-Zforce-unstable-if-unmarked")
+        cmd.arg("-Z")
+            .arg("force-unstable-if-unmarked")
             .env("RUSTC_BOOTSTRAP", "1");
     }
 
@@ -945,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<'_, '_>,

diff --git a/src/cargo/core/profiles.rs b/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

diff --git a/tests/testsuite/cache_messages.rs b/tests/testsuite/cache_messages.rs
@@ -238,12 +238,8 @@ fn rustdoc() {
         .file(
             "src/lib.rs",
             "
-            #![warn(private_doc_tests)]
-            /// asdf
-            /// ```
-            /// let x = 1;
-            /// ```
-            fn f() {}
+            #![warn(missing_docs)]
+            pub fn f() {}
             ",
         )
         .build();
@@ -254,7 +250,7 @@ fn rustdoc() {
         .expect("rustdoc to run");
     assert!(rustdoc_output.status.success());
     let rustdoc_stderr = as_str(&rustdoc_output.stderr);
-    assert!(rustdoc_stderr.contains("private"));
+    assert!(rustdoc_stderr.contains("missing"));
     assert!(rustdoc_stderr.contains("\x1b["));
     assert_eq!(
         p.glob("target/debug/.fingerprint/foo-*/output-*").count(),

diff --git a/tests/testsuite/lto.rs b/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]
@@ -25,7 +25,7 @@ fn with_deps() {
         .file("src/main.rs", "extern crate bar; fn main() {}")
         .build();
     p.cargo("build -v --release")
-        .with_stderr_contains("[..]`rustc[..]--crate-name bar[..]-Clinker-plugin-lto[..]`")
+        .with_stderr_contains("[..]`rustc[..]--crate-name bar[..]-C linker-plugin-lto[..]`")
         .with_stderr_contains("[..]`rustc[..]--crate-name test[..]-C lto[..]`")
         .run();
 }
@@ -83,7 +83,7 @@ fn build_dep_not_ltod() {
         .file("src/main.rs", "fn main() {}")
         .build();
     p.cargo("build -v --release")
-        .with_stderr_contains("[..]`rustc[..]--crate-name bar[..]-Cembed-bitcode=no[..]`")
+        .with_stderr_contains("[..]`rustc[..]--crate-name bar[..]-C embed-bitcode=no[..]`")
         .with_stderr_contains("[..]`rustc[..]--crate-name test[..]-C lto[..]`")
         .run();
 }
@@ -188,30 +188,32 @@ fn complicated() {
     p.cargo("build -v --release")
         // normal deps and their transitive dependencies do not need object
         // code, so they should have linker-plugin-lto specified
-        .with_stderr_contains("[..]`rustc[..]--crate-name dep_normal2 [..]-Clinker-plugin-lto[..]`")
-        .with_stderr_contains("[..]`rustc[..]--crate-name dep_normal [..]-Clinker-plugin-lto[..]`")
+        .with_stderr_contains(
+            "[..]`rustc[..]--crate-name dep_normal2 [..]-C linker-plugin-lto[..]`",
+        )
+        .with_stderr_contains("[..]`rustc[..]--crate-name dep_normal [..]-C linker-plugin-lto[..]`")
         // build dependencies and their transitive deps don't need any bitcode,
         // so embedding should be turned off
-        .with_stderr_contains("[..]`rustc[..]--crate-name dep_build2 [..]-Cembed-bitcode=no[..]`")
-        .with_stderr_contains("[..]`rustc[..]--crate-name dep_build [..]-Cembed-bitcode=no[..]`")
+        .with_stderr_contains("[..]`rustc[..]--crate-name dep_build2 [..]-C embed-bitcode=no[..]`")
+        .with_stderr_contains("[..]`rustc[..]--crate-name dep_build [..]-C embed-bitcode=no[..]`")
         .with_stderr_contains(
-            "[..]`rustc[..]--crate-name build_script_build [..]-Cembed-bitcode=no[..]`",
+            "[..]`rustc[..]--crate-name build_script_build [..]-C embed-bitcode=no[..]`",
         )
         // proc macro deps are the same as build deps here
         .with_stderr_contains(
-            "[..]`rustc[..]--crate-name dep_proc_macro2 [..]-Cembed-bitcode=no[..]`",
+            "[..]`rustc[..]--crate-name dep_proc_macro2 [..]-C embed-bitcode=no[..]`",
         )
         .with_stderr_contains(
-            "[..]`rustc[..]--crate-name dep_proc_macro [..]-Cembed-bitcode=no[..]`",
+            "[..]`rustc[..]--crate-name dep_proc_macro [..]-C embed-bitcode=no[..]`",
         )
         .with_stderr_contains("[..]`rustc[..]--crate-name test [..]--crate-type bin[..]-C lto[..]`")
         .with_stderr_contains(
             "[..]`rustc[..]--crate-name test [..]--crate-type cdylib[..]-C lto[..]`",
         )
         .with_stderr_contains("[..]`rustc[..]--crate-name dep_shared [..]`")
         .with_stderr_does_not_contain("[..]--crate-name dep_shared[..]-C lto[..]")
-        .with_stderr_does_not_contain("[..]--crate-name dep_shared[..]-Clinker-plugin-lto[..]")
-        .with_stderr_does_not_contain("[..]--crate-name dep_shared[..]-Cembed-bitcode[..]")
+        .with_stderr_does_not_contain("[..]--crate-name dep_shared[..]-C linker-plugin-lto[..]")
+        .with_stderr_does_not_contain("[..]--crate-name dep_shared[..]-C embed-bitcode[..]")
         .run();
 }
 
@@ -252,9 +254,9 @@ fn off_in_manifest_works() {
 [DOWNLOADING] [..]
 [DOWNLOADED] [..]
 [COMPILING] bar v0.0.1
-[RUNNING] `rustc --crate-name bar [..]--crate-type lib [..]-Cembed-bitcode=no[..]
+[RUNNING] `rustc --crate-name bar [..]--crate-type lib [..]-C embed-bitcode=no[..]
 [COMPILING] test [..]
-[RUNNING] `rustc --crate-name test [..]--crate-type lib [..]-Cembed-bitcode=no[..]
+[RUNNING] `rustc --crate-name test [..]--crate-type lib [..]-C embed-bitcode=no[..]
 [RUNNING] `rustc --crate-name test src/main.rs [..]--crate-type bin [..]-C lto=off[..]
 [FINISHED] [..]
 ",
@@ -283,7 +285,7 @@ fn between_builds() {
         .with_stderr(
             "\
 [COMPILING] test [..]
-[RUNNING] `rustc [..]--crate-type lib[..]-Clinker-plugin-lto[..]
+[RUNNING] `rustc [..]--crate-type lib[..]-C linker-plugin-lto[..]
 [FINISHED] [..]
 ",
         )
@@ -447,9 +449,9 @@ fn verify_lto(output: &Output, krate: &str, krate_info: &str, expected_lto: Lto)
         }
     } else if line.contains("-C lto") {
         Lto::Run(None)
-    } else if line.contains("-Clinker-plugin-lto") {
+    } else if line.contains("-C linker-plugin-lto") {
         Lto::OnlyBitcode
-    } else if line.contains("-Cembed-bitcode=no") {
+    } else if line.contains("-C embed-bitcode=no") {
         Lto::OnlyObject
     } else {
         Lto::ObjectAndBitcode
@@ -491,8 +493,8 @@ fn cdylib_and_rlib() {
 [FRESH] registry-shared v0.0.1
 [FRESH] bar v0.0.0 [..]
 [COMPILING] foo [..]
-[RUNNING] `rustc --crate-name foo [..]-Cembed-bitcode=no --test[..]
-[RUNNING] `rustc --crate-name a [..]-Cembed-bitcode=no --test[..]
+[RUNNING] `rustc --crate-name foo [..]-C embed-bitcode=no --test[..]
+[RUNNING] `rustc --crate-name a [..]-C embed-bitcode=no --test[..]
 [FINISHED] [..]
 [RUNNING] [..]
 [RUNNING] [..]
@@ -512,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 [..]-Cembed-bitcode=no --test[..]
-[RUNNING] `rustc --crate-name b [..]-Cembed-bitcode=no --test[..]
+[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();
@@ -547,8 +552,8 @@ fn dylib() {
 [FRESH] registry-shared v0.0.1
 [FRESH] bar v0.0.0 [..]
 [COMPILING] foo [..]
-[RUNNING] `rustc --crate-name foo [..]-Cembed-bitcode=no --test[..]
-[RUNNING] `rustc --crate-name a [..]-Cembed-bitcode=no --test[..]
+[RUNNING] `rustc --crate-name foo [..]-C embed-bitcode=no --test[..]
+[RUNNING] `rustc --crate-name a [..]-C embed-bitcode=no --test[..]
 [FINISHED] [..]
 [RUNNING] [..]
 [RUNNING] [..]
@@ -560,9 +565,9 @@ fn dylib() {
             "\
 [COMPILING] registry-shared v0.0.1
 [FRESH] registry v0.0.1
-[RUNNING] `rustc --crate-name registry_shared [..]-Cembed-bitcode=no[..]
+[RUNNING] `rustc --crate-name registry_shared [..]-C embed-bitcode=no[..]
 [COMPILING] bar [..]
-[RUNNING] `rustc --crate-name bar [..]--crate-type dylib [..]-Cembed-bitcode=no[..]
+[RUNNING] `rustc --crate-name bar [..]--crate-type dylib [..]-C embed-bitcode=no[..]
 [FINISHED] [..]
 ",
         )
@@ -573,8 +578,8 @@ fn dylib() {
 [FRESH] registry-shared v0.0.1
 [FRESH] registry v0.0.1
 [COMPILING] bar [..]
-[RUNNING] `rustc --crate-name bar [..]-Cembed-bitcode=no --test[..]
-[RUNNING] `rustc --crate-name b [..]-Cembed-bitcode=no --test[..]
+[RUNNING] `rustc --crate-name bar [..]-C embed-bitcode=no --test[..]
+[RUNNING] `rustc --crate-name b [..]-C embed-bitcode=no --test[..]
 [FINISHED] [..]
 [RUNNING] [..]
 [RUNNING] [..]
@@ -625,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 -Cembed-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] [..]
@@ -678,12 +683,66 @@ 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 -Clinker-plugin-lto [..]
-[RUNNING] `rustc --crate-name foo [..]--emit=dep-info,link -Cembed-bitcode=no [..]--test[..]
+[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] [..]
 [DOCTEST] foo
 [RUNNING] `rustdoc [..]
 ")
         .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();
+}