Handle LTO with an rlib/cdylib crate type

src/cargo/core/compiler/lto.rs

@@ -1,6 +1,7 @@
 use crate::core::compiler::{Context, Unit};
 use crate::core::interning::InternedString;
 use crate::core::profiles;
+use crate::core::TargetKind;
 use crate::util::errors::CargoResult;
 use std::collections::hash_map::{Entry, HashMap};
 
@@ -26,7 +27,7 @@ pub enum Lto {
 pub fn generate(cx: &mut Context<'_, '_>) -> CargoResult<()> {
     let mut map = HashMap::new();
     for unit in cx.bcx.roots.iter() {
-        calculate(cx, &mut map, unit, false)?;
+        calculate(cx, &mut map, unit, Lto::None)?;
     }
     cx.lto = map;
     Ok(())
@@ -36,34 +37,49 @@ fn calculate(
     cx: &Context<'_, '_>,
     map: &mut HashMap<Unit, Lto>,
     unit: &Unit,
-    require_bitcode: bool,
+    lto_for_deps: Lto,
 ) -> CargoResult<()> {
-    let (lto, require_bitcode_for_deps) = if unit.target.for_host() {
+    let (lto, lto_for_deps) = if unit.target.for_host() {
         // Disable LTO for host builds since we only really want to perform LTO
         // for the final binary, and LTO on plugins/build scripts/proc macros is
         // largely not desired.
-        (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. Note that we ignore `require_bitcode`
+        (Lto::None, Lto::None)
+    } else if unit.target.is_linkable() {
+        // A "linkable" target is one that produces and rlib or dylib in this
+        // case. In this scenario we cannot pass `-Clto` to the compiler because
+        // that is an invalid request, this is simply a dependency. What we do,
+        // however, is respect the request for whatever dependencies need to
+        // have.
+        //
+        // Here if no LTO is requested then we keep it turned off. Otherwise LTO
+        // is requested in some form, which means ideally we need just what's
+        // requested, nothing else. It's possible, though, to have libraries
+        // which are both a cdylib and and rlib, for example, which means that
+        // object files are getting sent to the linker. That means that we need
+        // to fully embed bitcode rather than simply generating just bitcode.
+        let has_non_linkable_lib = match unit.target.kind() {
+            TargetKind::Lib(kinds) => kinds.iter().any(|k| !k.is_linkable()),
+            _ => true,
+        };
+        match lto_for_deps {
+            Lto::None => (Lto::None, Lto::None),
+            _ if has_non_linkable_lib => (Lto::EmbedBitcode, Lto::EmbedBitcode),
+            other => (other, other),
+        }
+    } else {
+        // Otherwise this target can perform LTO and we're going to read the
+        // LTO value out of the profile. Note that we ignore `lto_for_deps`
         // 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),
-                _ => (Lto::Run(Some(s)), true),
+                "n" | "no" | "off" => (Lto::Run(Some(s)), Lto::None),
+                _ => (Lto::Run(Some(s)), Lto::OnlyBitcode),
             },
-            profiles::Lto::Bool(true) => (Lto::Run(None), true),
-            profiles::Lto::Bool(false) => (Lto::None, false),
+            profiles::Lto::Bool(true) => (Lto::Run(None), Lto::OnlyBitcode),
+            profiles::Lto::Bool(false) => (Lto::None, Lto::None),
         }
-    } else if require_bitcode {
-        // Otherwise we're a dependency of something, an rlib. This means that
-        // if our parent required bitcode of some kind then we need to generate
-        // bitcode.
-        (Lto::OnlyBitcode, true)
-    } else {
-        (Lto::None, false)
     };
 
     match map.entry(unit.clone()) {
@@ -93,14 +109,12 @@ fn calculate(
                 // either only-objects or only-bitcode we have to embed both in
                 // rlibs (used for different compilations), so we switch to
                 // embedding bitcode.
-                (Lto::OnlyBitcode, Lto::None)
-                | (Lto::OnlyBitcode, Lto::EmbedBitcode)
-                | (Lto::None, Lto::OnlyBitcode)
-                | (Lto::None, Lto::EmbedBitcode) => Lto::EmbedBitcode,
+                (Lto::OnlyBitcode, Lto::None) | (Lto::None, Lto::OnlyBitcode) => Lto::EmbedBitcode,
 
-                // Currently this variant is never calculated above, so no need
-                // to handle this case.
-                (Lto::EmbedBitcode, _) => unreachable!(),
+                // Once a target has requested bitcode embedding that's the
+                // maximal amount of work that can be done, so we just keep
+                // doing that work.
+                (Lto::EmbedBitcode, _) | (_, Lto::EmbedBitcode) => Lto::EmbedBitcode,
             };
             // No need to recurse if we calculated the same value as before.
             if result == *v.get() {
@@ -111,7 +125,7 @@ fn calculate(
     }
 
     for dep in cx.unit_deps(unit) {
-        calculate(cx, map, &dep.unit, require_bitcode_for_deps)?;
+        calculate(cx, map, &dep.unit, lto_for_deps)?;
     }
     Ok(())
 }

src/cargo/core/manifest.rs

@@ -842,17 +842,6 @@ impl Target {
         self.kind().rustc_crate_types()
     }
 
-    pub fn can_lto(&self) -> bool {
-        match self.kind() {
-            TargetKind::Lib(v) => {
-                !v.contains(&CrateType::Rlib)
-                    && !v.contains(&CrateType::Dylib)
-                    && !v.contains(&CrateType::Lib)
-            }
-            _ => true,
-        }
-    }
-
     pub fn set_tested(&mut self, tested: bool) -> &mut Target {
         Arc::make_mut(&mut self.inner).tested = tested;
         self

tests/testsuite/lto.rs

@@ -336,3 +336,78 @@ fn test_all_and_bench() {
         .with_stderr_contains("[RUNNING] `rustc[..]--crate-name foo[..]-C lto[..]")
         .run();
 }
+
+#[cargo_test]
+fn cdylib_and_rlib() {
+    if !cargo_test_support::is_nightly() {
+        return;
+    }
+
+    Package::new("registry", "0.0.1")
+        .file("src/lib.rs", "pub fn foo() {}")
+        .publish();
+    Package::new("registry-shared", "0.0.1")
+        .file("src/lib.rs", "pub fn foo() {}")
+        .publish();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.0.0"
+
+                [workspace]
+
+                [dependencies]
+                bar = { path = 'bar' }
+                registry-shared = "*"
+
+                [profile.release]
+                lto = true
+            "#,
+        )
+        .file(
+            "src/main.rs",
+            "
+                fn main() {
+                    bar::foo();
+                    registry_shared::foo();
+                }
+            ",
+        )
+        .file(
+            "bar/Cargo.toml",
+            r#"
+                [package]
+                name = "bar"
+                version = "0.0.0"
+
+                [dependencies]
+                registry = "*"
+                registry-shared = "*"
+
+                [lib]
+                crate-type = ['cdylib', 'rlib']
+            "#,
+        )
+        .file(
+            "bar/src/lib.rs",
+            "
+                pub fn foo() {
+                    registry::foo();
+                    registry_shared::foo();
+                }
+            ",
+        )
+        .file("tests/a.rs", "")
+        .file("bar/tests/b.rs", "")
+        .build();
+    p.cargo("build --release -v").run();
+    p.cargo("test --release -v").run();
+    p.cargo("build --release -v --manifest-path bar/Cargo.toml")
+        .run();
+    p.cargo("test --release -v --manifest-path bar/Cargo.toml")
+        .run();
+}