Auto merge of #116014 - lqd:mcp510-2-electric-boogaloo, r=petrochenkov

Implement `-Clink-self-contained=-linker` opt out

This implements the `-Clink-self-contained` opt out necessary to switch to lld by changing rustc's defaults instead of cargo's.

Components that are enabled and disabled on the CLI are recorded, for the purpose of being merged with the ones which the target spec will declare (I'll open another PR for that tomorrow, for easier review).

For MCP510, we now check whether using the self-contained linker is disabled on the CLI. Right now it would only be sensible to with `-Zgcc-ld=lld` (and I'll add some checks that we don't both enable and disable a component on the CLI in a future PR), but the goal is to simplify adding the check of the target's enabled components here in the follow-up PRs.

r? `@petrochenkov`

Author: bors

Cargo.lock

@@ -4479,7 +4479,6 @@ dependencies = [
 name = "rustc_session"
 version = "0.0.0"
 dependencies = [
- "bitflags 1.3.2",
  "getopts",
  "libc",
  "rustc_ast",

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -2978,8 +2978,9 @@ fn add_lld_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
     }
 
     // 1. Implement the "self-contained" part of this feature by adding rustc distribution
-    //    directories to the tool's search path.
-    if sess.opts.cg.link_self_contained.linker() {
+    // directories to the tool's search path:
+    // - if the self-contained linker is enabled on the CLI.
+    if sess.opts.cg.link_self_contained.is_linker_enabled() {
         for path in sess.get_tools_search_paths(false) {
             cmd.arg({
                 let mut arg = OsString::from("-B");
@@ -2990,7 +2991,7 @@ fn add_lld_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) {
     }
 
     // 2. Implement the "linker flavor" part of this feature by asking `cc` to use some kind of
-    //    `lld` as the linker.
+    // `lld` as the linker.
     cmd.arg("-fuse-ld=lld");
 
     if !flavor.is_gnu() {

compiler/rustc_session/Cargo.toml

@@ -4,7 +4,6 @@ version = "0.0.0"
 edition = "2021"
 
 [dependencies]
-bitflags = "1.2.1"
 getopts = "0.2"
 rustc_macros = { path = "../rustc_macros" }
 tracing = "0.1"

compiler/rustc_session/src/config.rs

@@ -12,6 +12,7 @@ use crate::{EarlyErrorHandler, Session};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::stable_hasher::{StableOrd, ToStableHashKey};
 use rustc_target::abi::Align;
+use rustc_target::spec::LinkSelfContainedComponents;
 use rustc_target::spec::{PanicStrategy, RelocModel, SanitizerSet, SplitDebuginfo};
 use rustc_target::spec::{Target, TargetTriple, TargetWarnings, TARGETS};
 
@@ -232,75 +233,50 @@ pub struct LinkSelfContained {
     /// Used for compatibility with the existing opt-in and target inference.
     pub explicitly_set: Option<bool>,
 
-    /// The components that are enabled.
-    components: LinkSelfContainedComponents,
-}
-
-bitflags::bitflags! {
-    #[derive(Default)]
-    /// The `-C link-self-contained` components that can individually be enabled or disabled.
-    pub struct LinkSelfContainedComponents: u8 {
-        /// CRT objects (e.g. on `windows-gnu`, `musl`, `wasi` targets)
-        const CRT_OBJECTS = 1 << 0;
-        /// libc static library (e.g. on `musl`, `wasi` targets)
-        const LIBC        = 1 << 1;
-        /// libgcc/libunwind (e.g. on `windows-gnu`, `fuchsia`, `fortanix`, `gnullvm` targets)
-        const UNWIND      = 1 << 2;
-        /// Linker, dlltool, and their necessary libraries (e.g. on `windows-gnu` and for `rust-lld`)
-        const LINKER      = 1 << 3;
-        /// Sanitizer runtime libraries
-        const SANITIZERS  = 1 << 4;
-        /// Other MinGW libs and Windows import libs
-        const MINGW       = 1 << 5;
-    }
-}
-
-impl FromStr for LinkSelfContainedComponents {
-    type Err = ();
+    /// The components that are enabled on the CLI, using the `+component` syntax or one of the
+    /// `true` shorcuts.
+    enabled_components: LinkSelfContainedComponents,
 
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Ok(match s {
-            "crto" => LinkSelfContainedComponents::CRT_OBJECTS,
-            "libc" => LinkSelfContainedComponents::LIBC,
-            "unwind" => LinkSelfContainedComponents::UNWIND,
-            "linker" => LinkSelfContainedComponents::LINKER,
-            "sanitizers" => LinkSelfContainedComponents::SANITIZERS,
-            "mingw" => LinkSelfContainedComponents::MINGW,
-            _ => return Err(()),
-        })
-    }
+    /// The components that are disabled on the CLI, using the `-component` syntax or one of the
+    /// `false` shortcuts.
+    disabled_components: LinkSelfContainedComponents,
 }
 
 impl LinkSelfContained {
     /// Incorporates an enabled or disabled component as specified on the CLI, if possible.
     /// For example: `+linker`, and `-crto`.
-    pub(crate) fn handle_cli_component(&mut self, component: &str) -> Result<(), ()> {
+    pub(crate) fn handle_cli_component(&mut self, component: &str) -> Option<()> {
         // Note that for example `-Cself-contained=y -Cself-contained=-linker` is not an explicit
         // set of all values like `y` or `n` used to be. Therefore, if this flag had previously been
         // set in bulk with its historical values, then manually setting a component clears that
         // `explicitly_set` state.
         if let Some(component_to_enable) = component.strip_prefix('+') {
             self.explicitly_set = None;
-            self.components.insert(component_to_enable.parse()?);
-            Ok(())
+            self.enabled_components
+                .insert(LinkSelfContainedComponents::from_str(component_to_enable)?);
+            Some(())
         } else if let Some(component_to_disable) = component.strip_prefix('-') {
             self.explicitly_set = None;
-            self.components.remove(component_to_disable.parse()?);
-            Ok(())
+            self.disabled_components
+                .insert(LinkSelfContainedComponents::from_str(component_to_disable)?);
+            Some(())
         } else {
-            Err(())
+            None
         }
     }
 
     /// Turns all components on or off and records that this was done explicitly for compatibility
     /// purposes.
     pub(crate) fn set_all_explicitly(&mut self, enabled: bool) {
         self.explicitly_set = Some(enabled);
-        self.components = if enabled {
-            LinkSelfContainedComponents::all()
+
+        if enabled {
+            self.enabled_components = LinkSelfContainedComponents::all();
+            self.disabled_components = LinkSelfContainedComponents::empty();
         } else {
-            LinkSelfContainedComponents::empty()
-        };
+            self.enabled_components = LinkSelfContainedComponents::empty();
+            self.disabled_components = LinkSelfContainedComponents::all();
+        }
     }
 
     /// Helper creating a fully enabled `LinkSelfContained` instance. Used in tests.
@@ -314,13 +290,32 @@ impl LinkSelfContained {
     /// components was set individually. This would also require the `-Zunstable-options` flag, to
     /// be allowed.
     fn are_unstable_variants_set(&self) -> bool {
-        let any_component_set = !self.components.is_empty();
+        let any_component_set =
+            !self.enabled_components.is_empty() || !self.disabled_components.is_empty();
         self.explicitly_set.is_none() && any_component_set
     }
 
-    /// Returns whether the self-contained linker component is enabled.
-    pub fn linker(&self) -> bool {
-        self.components.contains(LinkSelfContainedComponents::LINKER)
+    /// Returns whether the self-contained linker component was enabled on the CLI, using the
+    /// `-C link-self-contained=+linker` syntax, or one of the `true` shorcuts.
+    pub fn is_linker_enabled(&self) -> bool {
+        self.enabled_components.contains(LinkSelfContainedComponents::LINKER)
+    }
+
+    /// Returns whether the self-contained linker component was disabled on the CLI, using the
+    /// `-C link-self-contained=-linker` syntax, or one of the `false` shorcuts.
+    pub fn is_linker_disabled(&self) -> bool {
+        self.disabled_components.contains(LinkSelfContainedComponents::LINKER)
+    }
+
+    /// Returns CLI inconsistencies to emit errors: individual components were both enabled and
+    /// disabled.
+    fn check_consistency(&self) -> Option<LinkSelfContainedComponents> {
+        if self.explicitly_set.is_some() {
+            None
+        } else {
+            let common = self.enabled_components.intersection(self.disabled_components);
+            if common.is_empty() { None } else { Some(common) }
+        }
     }
 }
 
@@ -2758,9 +2753,8 @@ pub fn build_session_options(
     }
 
     // For testing purposes, until we have more feedback about these options: ensure `-Z
-    // unstable-options` is required when using the unstable `-C link-self-contained` options, like
-    // `-C link-self-contained=+linker`, and when using the unstable `-C linker-flavor` options, like
-    // `-C linker-flavor=gnu-lld-cc`.
+    // unstable-options` is required when using the unstable `-C link-self-contained` and `-C
+    // linker-flavor` options.
     if !nightly_options::is_unstable_enabled(matches) {
         let uses_unstable_self_contained_option =
             cg.link_self_contained.are_unstable_variants_set();
@@ -2782,6 +2776,19 @@ pub fn build_session_options(
         }
     }
 
+    // Check `-C link-self-contained` for consistency: individual components cannot be both enabled
+    // and disabled at the same time.
+    if let Some(erroneous_components) = cg.link_self_contained.check_consistency() {
+        let names: String = erroneous_components
+            .into_iter()
+            .map(|c| c.as_str().unwrap())
+            .intersperse(", ")
+            .collect();
+        handler.early_error(format!(
+            "some `-C link-self-contained` components were both enabled and disabled: {names}"
+        ));
+    }
+
     let prints = collect_print_requests(handler, &mut cg, &mut unstable_opts, matches);
 
     let cg = cg;

compiler/rustc_session/src/lib.rs

@@ -6,6 +6,7 @@
 #![feature(option_get_or_insert_default)]
 #![feature(rustc_attrs)]
 #![feature(map_many_mut)]
+#![feature(iter_intersperse)]
 #![recursion_limit = "256"]
 #![allow(rustc::potential_query_instability)]
 #![deny(rustc::untranslatable_diagnostic)]

compiler/rustc_session/src/options.rs

@@ -1165,7 +1165,7 @@ mod parse {
 
         // 2. Parse a list of enabled and disabled components.
         for comp in s.split(',') {
-            if slot.handle_cli_component(comp).is_err() {
+            if slot.handle_cli_component(comp).is_none() {
                 return false;
             }
         }

compiler/rustc_target/src/spec/mod.rs

@@ -164,11 +164,11 @@ pub enum LinkerFlavor {
 
 /// Linker flavors available externally through command line (`-Clinker-flavor`)
 /// or json target specifications.
-/// FIXME: This set has accumulated historically, bring it more in line with the internal
-/// linker flavors (`LinkerFlavor`).
+/// This set has accumulated historically, and contains both (stable and unstable) legacy values, as
+/// well as modern ones matching the internal linker flavors (`LinkerFlavor`).
 #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
 pub enum LinkerFlavorCli {
-    // New (unstable) flavors, with direct counterparts in `LinkerFlavor`.
+    // Modern (unstable) flavors, with direct counterparts in `LinkerFlavor`.
     Gnu(Cc, Lld),
     Darwin(Cc, Lld),
     WasmLld(Cc),
@@ -179,7 +179,7 @@ pub enum LinkerFlavorCli {
     Bpf,
     Ptx,
 
-    // Below: the legacy stable values.
+    // Legacy stable values
     Gcc,
     Ld,
     Lld(LldFlavor),
@@ -504,7 +504,7 @@ linker_flavor_cli_impls! {
     (LinkerFlavorCli::Bpf) "bpf"
     (LinkerFlavorCli::Ptx) "ptx"
 
-    // Below: legacy stable values
+    // Legacy stable flavors
     (LinkerFlavorCli::Gcc) "gcc"
     (LinkerFlavorCli::Ld) "ld"
     (LinkerFlavorCli::Lld(LldFlavor::Ld)) "ld.lld"
@@ -520,6 +520,80 @@ impl ToJson for LinkerFlavorCli {
     }
 }
 
+bitflags::bitflags! {
+    #[derive(Default)]
+    /// The `-C link-self-contained` components that can individually be enabled or disabled.
+    pub struct LinkSelfContainedComponents: u8 {
+        /// CRT objects (e.g. on `windows-gnu`, `musl`, `wasi` targets)
+        const CRT_OBJECTS = 1 << 0;
+        /// libc static library (e.g. on `musl`, `wasi` targets)
+        const LIBC        = 1 << 1;
+        /// libgcc/libunwind (e.g. on `windows-gnu`, `fuchsia`, `fortanix`, `gnullvm` targets)
+        const UNWIND      = 1 << 2;
+        /// Linker, dlltool, and their necessary libraries (e.g. on `windows-gnu` and for `rust-lld`)
+        const LINKER      = 1 << 3;
+        /// Sanitizer runtime libraries
+        const SANITIZERS  = 1 << 4;
+        /// Other MinGW libs and Windows import libs
+        const MINGW       = 1 << 5;
+    }
+}
+
+impl LinkSelfContainedComponents {
+    /// Parses a single `-Clink-self-contained` well-known component, not a set of flags.
+    pub fn from_str(s: &str) -> Option<LinkSelfContainedComponents> {
+        Some(match s {
+            "crto" => LinkSelfContainedComponents::CRT_OBJECTS,
+            "libc" => LinkSelfContainedComponents::LIBC,
+            "unwind" => LinkSelfContainedComponents::UNWIND,
+            "linker" => LinkSelfContainedComponents::LINKER,
+            "sanitizers" => LinkSelfContainedComponents::SANITIZERS,
+            "mingw" => LinkSelfContainedComponents::MINGW,
+            _ => return None,
+        })
+    }
+
+    /// Return the component's name.
+    ///
+    /// Returns `None` if the bitflags aren't a singular component (but a mix of multiple flags).
+    pub fn as_str(self) -> Option<&'static str> {
+        Some(match self {
+            LinkSelfContainedComponents::CRT_OBJECTS => "crto",
+            LinkSelfContainedComponents::LIBC => "libc",
+            LinkSelfContainedComponents::UNWIND => "unwind",
+            LinkSelfContainedComponents::LINKER => "linker",
+            LinkSelfContainedComponents::SANITIZERS => "sanitizers",
+            LinkSelfContainedComponents::MINGW => "mingw",
+            _ => return None,
+        })
+    }
+
+    /// Returns an array of all the components.
+    fn all_components() -> [LinkSelfContainedComponents; 6] {
+        [
+            LinkSelfContainedComponents::CRT_OBJECTS,
+            LinkSelfContainedComponents::LIBC,
+            LinkSelfContainedComponents::UNWIND,
+            LinkSelfContainedComponents::LINKER,
+            LinkSelfContainedComponents::SANITIZERS,
+            LinkSelfContainedComponents::MINGW,
+        ]
+    }
+}
+
+impl IntoIterator for LinkSelfContainedComponents {
+    type Item = LinkSelfContainedComponents;
+    type IntoIter = std::vec::IntoIter<LinkSelfContainedComponents>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        LinkSelfContainedComponents::all_components()
+            .into_iter()
+            .filter(|&s| self.contains(s))
+            .collect::<Vec<_>>()
+            .into_iter()
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Hash, Encodable, Decodable, HashStable_Generic)]
 pub enum PanicStrategy {
     Unwind,

tests/ui/linkage-attr/link-self-contained-consistency.many.stderr

@@ -0,0 +1,2 @@
+error: some `-C link-self-contained` components were both enabled and disabled: crto, linker
+

tests/ui/linkage-attr/link-self-contained-consistency.one.stderr

@@ -0,0 +1,2 @@
+error: some `-C link-self-contained` components were both enabled and disabled: linker
+

tests/ui/linkage-attr/link-self-contained-consistency.rs

@@ -0,0 +1,10 @@
+// Checks that self-contained linking components cannot be both enabled and disabled at the same
+// time on the CLI.
+
+// check-fail
+// revisions: one many
+// [one] compile-flags: -Clink-self-contained=-linker -Clink-self-contained=+linker -Zunstable-options
+// [many] compile-flags: -Clink-self-contained=+linker,+crto -Clink-self-contained=-linker,-crto -Zunstable-options
+// ignore-tidy-linelength
+
+fn main() {}