Auto merge of rust-lang#96885 - petrochenkov:linkstrict2, r=cjgillot,luqmana

rustc: Stricter checking for #[link] attributes

A subset of rust-lang#94962 that doesn't touch library renaming/reordering/deduplication.

`#[link]` attributes are checked for all kinds of unexpected arguments inside them.
I also tried to make wording for these errors more consistent, that's why some existing errors are changed, including errors for command line `-l` options.
Spans are also made more precise where possible.

Author: bors

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -396,37 +396,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
             }
         }
 
-        // Check for unstable modifiers on `#[link(..)]` attribute
-        if attr.has_name(sym::link) {
-            for nested_meta in attr.meta_item_list().unwrap_or_default() {
-                if nested_meta.has_name(sym::modifiers) {
-                    if let Some(modifiers) = nested_meta.value_str() {
-                        for modifier in modifiers.as_str().split(',') {
-                            if let Some(modifier) = modifier.strip_prefix(&['+', '-']) {
-                                macro_rules! gate_modifier { ($($name:literal => $feature:ident)*) => {
-                                    $(if modifier == $name {
-                                        let msg = concat!("`#[link(modifiers=\"", $name, "\")]` is unstable");
-                                        gate_feature_post!(
-                                            self,
-                                            $feature,
-                                            nested_meta.name_value_literal_span().unwrap(),
-                                            msg
-                                        );
-                                    })*
-                                }}
-
-                                gate_modifier!(
-                                    "bundle" => native_link_modifiers_bundle
-                                    "verbatim" => native_link_modifiers_verbatim
-                                    "as-needed" => native_link_modifiers_as_needed
-                                );
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
         // Emit errors for non-staged-api crates.
         if !self.features.staged_api {
             if attr.has_name(sym::rustc_deprecated)

compiler/rustc_error_codes/src/error_codes/E0455.md

@@ -1,6 +1,11 @@
+Some linking kinds are target-specific and not supported on all platforms.
+
 Linking with `kind=framework` is only supported when targeting macOS,
 as frameworks are specific to that operating system.
 
+Similarly, `kind=raw-dylib` is only supported when targeting Windows-like
+platforms.
+
 Erroneous code example:
 
 ```ignore (should-compile_fail-but-cannot-doctest-conditionally-without-macos)

compiler/rustc_error_codes/src/error_codes/E0458.md

@@ -12,3 +12,4 @@ Please specify a valid "kind" value, from one of the following:
 * static
 * dylib
 * framework
+* raw-dylib

compiler/rustc_metadata/src/lib.rs

@@ -1,7 +1,9 @@
 #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
 #![feature(crate_visibility_modifier)]
+#![feature(decl_macro)]
 #![feature(drain_filter)]
 #![feature(generators)]
+#![feature(let_chains)]
 #![feature(let_else)]
 #![feature(nll)]
 #![feature(once_cell)]

compiler/rustc_metadata/src/native_libs.rs

@@ -23,6 +23,7 @@ use rustc_session::lint::builtin::{
 use rustc_session::parse::feature_err;
 use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::{Span, DUMMY_SP};
+use rustc_target::spec::abi::Abi;
 use std::collections::hash_map::Entry;
 
 pub(crate) fn target_from_impl_item<'tcx>(
@@ -1317,22 +1318,27 @@ impl CheckAttrVisitor<'_> {
 
     /// Checks if `#[link]` is applied to an item other than a foreign module.
     fn check_link(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) {
-        match target {
-            Target::ForeignMod => {}
-            _ => {
-                self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
-                    let mut diag = lint.build("attribute should be applied to an `extern` block");
-                    diag.warn(
-                        "this was previously accepted by the compiler but is \
-                         being phased out; it will become a hard error in \
-                         a future release!",
-                    );
+        if target == Target::ForeignMod
+            && let hir::Node::Item(item) = self.tcx.hir().get(hir_id)
+            && let Item { kind: ItemKind::ForeignMod { abi, .. }, .. } = item
+            && !matches!(abi, Abi::Rust | Abi::RustIntrinsic | Abi::PlatformIntrinsic)
+        {
+            return;
+        }
 
-                    diag.span_label(span, "not an `extern` block");
-                    diag.emit();
-                });
+        self.tcx.struct_span_lint_hir(UNUSED_ATTRIBUTES, hir_id, attr.span, |lint| {
+            let mut diag =
+                lint.build("attribute should be applied to an `extern` block with non-Rust ABI");
+            diag.warn(
+                "this was previously accepted by the compiler but is \
+                 being phased out; it will become a hard error in \
+                 a future release!",
+            );
+            if target != Target::ForeignMod {
+                diag.span_label(span, "not an `extern` block");
             }
-        }
+            diag.emit();
+        });
     }
 
     /// Checks if `#[link_name]` is applied to an item other than a foreign function or static.

compiler/rustc_passes/src/check_attr.rs

@@ -1937,33 +1937,27 @@ fn parse_native_lib_kind(
     };
 
     let kind = match kind {
-        "dylib" => NativeLibKind::Dylib { as_needed: None },
-        "framework" => NativeLibKind::Framework { as_needed: None },
         "static" => NativeLibKind::Static { bundle: None, whole_archive: None },
         "static-nobundle" => {
             early_warn(
                 error_format,
                 "library kind `static-nobundle` has been superseded by specifying \
-                `-bundle` on library kind `static`. Try `static:-bundle`",
+                 modifier `-bundle` with library kind `static`. Try `static:-bundle`",
             );
-            if modifiers.is_some() {
-                early_error(
-                    error_format,
-                    "linking modifier can't be used with library kind `static-nobundle`",
-                )
-            }
             if !nightly_options::match_is_nightly_build(matches) {
                 early_error(
                     error_format,
-                    "library kind `static-nobundle` are currently unstable and only accepted on \
-                the nightly compiler",
+                    "library kind `static-nobundle` is unstable \
+                     and only accepted on the nightly compiler",
                 );
             }
             NativeLibKind::Static { bundle: Some(false), whole_archive: None }
         }
-        s => early_error(
+        "dylib" => NativeLibKind::Dylib { as_needed: None },
+        "framework" => NativeLibKind::Framework { as_needed: None },
+        _ => early_error(
             error_format,
-            &format!("unknown library kind `{s}`, expected one of dylib, framework, or static"),
+            &format!("unknown library kind `{kind}`, expected one of: static, dylib, framework"),
         ),
     };
     match modifiers {
@@ -1978,94 +1972,83 @@ fn parse_native_lib_modifiers(
     error_format: ErrorOutputType,
     matches: &getopts::Matches,
 ) -> (NativeLibKind, Option<bool>) {
-    let report_unstable_modifier = |modifier| {
-        if !nightly_options::is_unstable_enabled(matches) {
-            let why = if nightly_options::match_is_nightly_build(matches) {
-                " and only accepted on the nightly compiler"
-            } else {
-                ", the `-Z unstable-options` flag must also be passed to use it"
-            };
-            early_error(
-                error_format,
-                &format!("{modifier} linking modifier is currently unstable{why}"),
-            )
-        }
-    };
-
-    let mut has_duplicate_modifiers = false;
     let mut verbatim = None;
     for modifier in modifiers.split(',') {
         let (modifier, value) = match modifier.strip_prefix(&['+', '-']) {
             Some(m) => (m, modifier.starts_with('+')),
             None => early_error(
                 error_format,
                 "invalid linking modifier syntax, expected '+' or '-' prefix \
-                    before one of: bundle, verbatim, whole-archive, as-needed",
+                 before one of: bundle, verbatim, whole-archive, as-needed",
             ),
         };
 
+        let report_unstable_modifier = || {
+            if !nightly_options::is_unstable_enabled(matches) {
+                let why = if nightly_options::match_is_nightly_build(matches) {
+                    " and only accepted on the nightly compiler"
+                } else {
+                    ", the `-Z unstable-options` flag must also be passed to use it"
+                };
+                early_error(
+                    error_format,
+                    &format!("linking modifier `{modifier}` is unstable{why}"),
+                )
+            }
+        };
+        let assign_modifier = |dst: &mut Option<bool>| {
+            if dst.is_some() {
+                let msg = format!("multiple `{modifier}` modifiers in a single `-l` option");
+                early_error(error_format, &msg)
+            } else {
+                *dst = Some(value);
+            }
+        };
         match (modifier, &mut kind) {
             ("bundle", NativeLibKind::Static { bundle, .. }) => {
-                report_unstable_modifier(modifier);
-                if bundle.is_some() {
-                    has_duplicate_modifiers = true;
-                }
-                *bundle = Some(value);
+                report_unstable_modifier();
+                assign_modifier(bundle)
             }
             ("bundle", _) => early_error(
                 error_format,
-                "bundle linking modifier is only compatible with \
-                    `static` linking kind",
+                "linking modifier `bundle` is only compatible with `static` linking kind",
             ),
 
             ("verbatim", _) => {
-                report_unstable_modifier(modifier);
-                if verbatim.is_some() {
-                    has_duplicate_modifiers = true;
-                }
-                verbatim = Some(value);
+                report_unstable_modifier();
+                assign_modifier(&mut verbatim)
             }
 
             ("whole-archive", NativeLibKind::Static { whole_archive, .. }) => {
-                if whole_archive.is_some() {
-                    has_duplicate_modifiers = true;
-                }
-                *whole_archive = Some(value);
+                assign_modifier(whole_archive)
             }
             ("whole-archive", _) => early_error(
                 error_format,
-                "whole-archive linking modifier is only compatible with \
-                    `static` linking kind",
+                "linking modifier `whole-archive` is only compatible with `static` linking kind",
             ),
 
             ("as-needed", NativeLibKind::Dylib { as_needed })
             | ("as-needed", NativeLibKind::Framework { as_needed }) => {
-                report_unstable_modifier(modifier);
-                if as_needed.is_some() {
-                    has_duplicate_modifiers = true;
-                }
-                *as_needed = Some(value);
+                report_unstable_modifier();
+                assign_modifier(as_needed)
             }
             ("as-needed", _) => early_error(
                 error_format,
-                "as-needed linking modifier is only compatible with \
-                    `dylib` and `framework` linking kinds",
+                "linking modifier `as-needed` is only compatible with \
+                 `dylib` and `framework` linking kinds",
             ),
 
             // Note: this error also excludes the case with empty modifier
             // string, like `modifiers = ""`.
             _ => early_error(
                 error_format,
                 &format!(
-                    "unrecognized linking modifier `{modifier}`, expected one \
-                    of: bundle, verbatim, whole-archive, as-needed"
+                    "unknown linking modifier `{modifier}`, expected one \
+                     of: bundle, verbatim, whole-archive, as-needed"
                 ),
             ),
         }
     }
-    if has_duplicate_modifiers {
-        report_unstable_modifier("duplicating")
-    }
 
     (kind, verbatim)
 }
@@ -2093,6 +2076,9 @@ fn parse_libs(matches: &getopts::Matches, error_format: ErrorOutputType) -> Vec<
                 None => (name, None),
                 Some((name, new_name)) => (name.to_string(), Some(new_name.to_owned())),
             };
+            if name.is_empty() {
+                early_error(error_format, "library name must not be empty");
+            }
             NativeLib { name, new_name, kind, verbatim }
         })
         .collect()

compiler/rustc_session/src/config.rs

@@ -52,7 +52,8 @@ where `KIND` may be one of:
 If the kind is specified, then linking modifiers can be attached to it.
 Modifiers are specified as a comma-delimited string with each modifier prefixed with
 either a `+` or `-` to indicate that the modifier is enabled or disabled, respectively.
-The last boolean value specified for a given modifier wins. \
+Specifying multiple `modifiers` arguments in a single `link` attribute,
+or multiple identical modifiers in the same `modifiers` argument is not currently supported. \
 Example: `-l static:+whole-archive=mylib`.
 
 The kind of library and the modifiers can also be specified in a [`#[link]`

src/doc/rustc/src/command-line-arguments.md

@@ -1,4 +1,4 @@
-#[link(name = "")] //~ ERROR: given with empty name
+#[link(name = "")] //~ ERROR: link name must not be empty
 extern "C" {}
 
 fn main() {}

src/test/ui/empty/empty-linkname.rs

@@ -1,8 +1,8 @@
-error[E0454]: `#[link(name = "")]` given with empty name
-  --> $DIR/empty-linkname.rs:1:1
+error[E0454]: link name must not be empty
+  --> $DIR/empty-linkname.rs:1:15
    |
 LL | #[link(name = "")]
-   | ^^^^^^^^^^^^^^^^^^ empty name given
+   |               ^^ empty link name
 
 error: aborting due to previous error