Rollup merge of #142876 - JonathanBrouwer:target_feature_parser, r=oli-obk

Port `#[target_feature]` to new attribute parsing infrastructure

Ports `target_feature` to the new attribute parsing infrastructure for https://github.com/rust-lang/rust/issues/131229#issuecomment-2971353197

r? ``@jdonszelmann``

4 files changed, 122 insertions, 134 deletions

diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl
index 63f47b9b7de..63e9005da45 100644
--- a/compiler/rustc_codegen_ssa/messages.ftl
+++ b/compiler/rustc_codegen_ssa/messages.ftl
@@ -62,6 +62,10 @@ codegen_ssa_failed_to_get_layout = failed to get layout for {$ty}: {$err}
 
 codegen_ssa_failed_to_write = failed to write {$path}: {$error}
 
+codegen_ssa_feature_not_valid = the feature named `{$feature}` is not valid for this target
+    .label = `{$feature}` is not valid for this target
+    .help = consider removing the leading `+` in the feature name
+
 codegen_ssa_field_associated_value_expected = associated value expected for `{$name}`
 
 codegen_ssa_forbidden_ctarget_feature =
diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
index 6e2143858de..2713ec07f97 100644
--- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
+++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs
@@ -141,6 +141,49 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                         });
                     }
                 }
+                AttributeKind::TargetFeature(features, attr_span) => {
+                    let Some(sig) = tcx.hir_node_by_def_id(did).fn_sig() else {
+                        tcx.dcx().span_delayed_bug(*attr_span, "target_feature applied to non-fn");
+                        continue;
+                    };
+                    let safe_target_features =
+                        matches!(sig.header.safety, hir::HeaderSafety::SafeTargetFeatures);
+                    codegen_fn_attrs.safe_target_features = safe_target_features;
+                    if safe_target_features {
+                        if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
+                            // The `#[target_feature]` attribute is allowed on
+                            // WebAssembly targets on all functions. Prior to stabilizing
+                            // the `target_feature_11` feature, `#[target_feature]` was
+                            // only permitted on unsafe functions because on most targets
+                            // execution of instructions that are not supported is
+                            // considered undefined behavior. For WebAssembly which is a
+                            // 100% safe target at execution time it's not possible to
+                            // execute undefined instructions, and even if a future
+                            // feature was added in some form for this it would be a
+                            // deterministic trap. There is no undefined behavior when
+                            // executing WebAssembly so `#[target_feature]` is allowed
+                            // on safe functions (but again, only for WebAssembly)
+                            //
+                            // Note that this is also allowed if `actually_rustdoc` so
+                            // if a target is documenting some wasm-specific code then
+                            // it's not spuriously denied.
+                            //
+                            // Now that `#[target_feature]` is permitted on safe functions,
+                            // this exception must still exist for allowing the attribute on
+                            // `main`, `start`, and other functions that are not usually
+                            // allowed.
+                        } else {
+                            check_target_feature_trait_unsafe(tcx, did, *attr_span);
+                        }
+                    }
+                    from_target_feature_attr(
+                        tcx,
+                        did,
+                        features,
+                        rust_target_features,
+                        &mut codegen_fn_attrs.target_features,
+                    );
+                }
                 AttributeKind::TrackCaller(attr_span) => {
                     let is_closure = tcx.is_closure_like(did.to_def_id());
 
@@ -190,49 +233,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
                 codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL
             }
             sym::thread_local => codegen_fn_attrs.flags |= CodegenFnAttrFlags::THREAD_LOCAL,
-            sym::target_feature => {
-                let Some(sig) = tcx.hir_node_by_def_id(did).fn_sig() else {
-                    tcx.dcx().span_delayed_bug(attr.span(), "target_feature applied to non-fn");
-                    continue;
-                };
-                let safe_target_features =
-                    matches!(sig.header.safety, hir::HeaderSafety::SafeTargetFeatures);
-                codegen_fn_attrs.safe_target_features = safe_target_features;
-                if safe_target_features {
-                    if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
-                        // The `#[target_feature]` attribute is allowed on
-                        // WebAssembly targets on all functions. Prior to stabilizing
-                        // the `target_feature_11` feature, `#[target_feature]` was
-                        // only permitted on unsafe functions because on most targets
-                        // execution of instructions that are not supported is
-                        // considered undefined behavior. For WebAssembly which is a
-                        // 100% safe target at execution time it's not possible to
-                        // execute undefined instructions, and even if a future
-                        // feature was added in some form for this it would be a
-                        // deterministic trap. There is no undefined behavior when
-                        // executing WebAssembly so `#[target_feature]` is allowed
-                        // on safe functions (but again, only for WebAssembly)
-                        //
-                        // Note that this is also allowed if `actually_rustdoc` so
-                        // if a target is documenting some wasm-specific code then
-                        // it's not spuriously denied.
-                        //
-                        // Now that `#[target_feature]` is permitted on safe functions,
-                        // this exception must still exist for allowing the attribute on
-                        // `main`, `start`, and other functions that are not usually
-                        // allowed.
-                    } else {
-                        check_target_feature_trait_unsafe(tcx, did, attr.span());
-                    }
-                }
-                from_target_feature_attr(
-                    tcx,
-                    did,
-                    attr,
-                    rust_target_features,
-                    &mut codegen_fn_attrs.target_features,
-                );
-            }
             sym::linkage => {
                 if let Some(val) = attr.value_str() {
                     let linkage = Some(linkage_by_name(tcx, did, val.as_str()));
@@ -536,10 +536,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
             .map(|features| (features.name.as_str(), true))
             .collect(),
     ) {
-        let span = tcx
-            .get_attrs(did, sym::target_feature)
-            .next()
-            .map_or_else(|| tcx.def_span(did), |a| a.span());
+        let span =
+            find_attr!(tcx.get_all_attrs(did), AttributeKind::TargetFeature(_, span) => *span)
+                .unwrap_or_else(|| tcx.def_span(did));
+
         tcx.dcx()
             .create_err(errors::TargetFeatureDisableOrEnable {
                 features,
diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs
index 1950a35b364..086c069745c 100644
--- a/compiler/rustc_codegen_ssa/src/errors.rs
+++ b/compiler/rustc_codegen_ssa/src/errors.rs
@@ -1292,3 +1292,14 @@ pub(crate) struct NoMangleNameless {
     pub span: Span,
     pub definition: String,
 }
+
+#[derive(Diagnostic)]
+#[diag(codegen_ssa_feature_not_valid)]
+pub(crate) struct FeatureNotValid<'a> {
+    pub feature: &'a str,
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    #[help]
+    pub plus_hint: bool,
+}
diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs
index faa278cc766..53df99993f0 100644
--- a/compiler/rustc_codegen_ssa/src/target_features.rs
+++ b/compiler/rustc_codegen_ssa/src/target_features.rs
@@ -1,8 +1,6 @@
 use rustc_attr_data_structures::InstructionSetAttr;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
 use rustc_data_structures::unord::{UnordMap, UnordSet};
-use rustc_errors::Applicability;
-use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
 use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
@@ -12,110 +10,85 @@ use rustc_session::Session;
 use rustc_session::lint::builtin::AARCH64_SOFTFLOAT_NEON;
 use rustc_session::parse::feature_err;
 use rustc_span::{Span, Symbol, sym};
-use rustc_target::target_features::{self, RUSTC_SPECIFIC_FEATURES, Stability};
+use rustc_target::target_features::{RUSTC_SPECIFIC_FEATURES, Stability};
 use smallvec::SmallVec;
 
-use crate::errors;
+use crate::errors::FeatureNotValid;
+use crate::{errors, target_features};
 
 /// Compute the enabled target features from the `#[target_feature]` function attribute.
 /// Enabled target features are added to `target_features`.
 pub(crate) fn from_target_feature_attr(
     tcx: TyCtxt<'_>,
     did: LocalDefId,
-    attr: &hir::Attribute,
+    features: &[(Symbol, Span)],
     rust_target_features: &UnordMap<String, target_features::Stability>,
     target_features: &mut Vec<TargetFeature>,
 ) {
-    let Some(list) = attr.meta_item_list() else { return };
-    let bad_item = |span| {
-        let msg = "malformed `target_feature` attribute input";
-        let code = "enable = \"..\"";
-        tcx.dcx()
-            .struct_span_err(span, msg)
-            .with_span_suggestion(span, "must be of the form", code, Applicability::HasPlaceholders)
-            .emit();
-    };
     let rust_features = tcx.features();
     let abi_feature_constraints = tcx.sess.target.abi_required_features();
-    for item in list {
-        // Only `enable = ...` is accepted in the meta-item list.
-        if !item.has_name(sym::enable) {
-            bad_item(item.span());
-            continue;
-        }
-
-        // Must be of the form `enable = "..."` (a string).
-        let Some(value) = item.value_str() else {
-            bad_item(item.span());
+    for &(feature, feature_span) in features {
+        let feature_str = feature.as_str();
+        let Some(stability) = rust_target_features.get(feature_str) else {
+            let plus_hint = feature_str
+                .strip_prefix('+')
+                .is_some_and(|stripped| rust_target_features.contains_key(stripped));
+            tcx.dcx().emit_err(FeatureNotValid {
+                feature: feature_str,
+                span: feature_span,
+                plus_hint,
+            });
             continue;
         };
 
-        // We allow comma separation to enable multiple features.
-        for feature in value.as_str().split(',') {
-            let Some(stability) = rust_target_features.get(feature) else {
-                let msg = format!("the feature named `{feature}` is not valid for this target");
-                let mut err = tcx.dcx().struct_span_err(item.span(), msg);
-                err.span_label(item.span(), format!("`{feature}` is not valid for this target"));
-                if let Some(stripped) = feature.strip_prefix('+') {
-                    let valid = rust_target_features.contains_key(stripped);
-                    if valid {
-                        err.help("consider removing the leading `+` in the feature name");
-                    }
-                }
-                err.emit();
-                continue;
-            };
-
-            // Only allow target features whose feature gates have been enabled
-            // and which are permitted to be toggled.
-            if let Err(reason) = stability.toggle_allowed() {
-                tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
-                    span: item.span(),
-                    feature,
-                    reason,
-                });
-            } else if let Some(nightly_feature) = stability.requires_nightly()
-                && !rust_features.enabled(nightly_feature)
-            {
-                feature_err(
-                    &tcx.sess,
-                    nightly_feature,
-                    item.span(),
-                    format!("the target feature `{feature}` is currently unstable"),
-                )
-                .emit();
-            } else {
-                // Add this and the implied features.
-                let feature_sym = Symbol::intern(feature);
-                for &name in tcx.implied_target_features(feature_sym) {
-                    // But ensure the ABI does not forbid enabling this.
-                    // Here we do assume that the backend doesn't add even more implied features
-                    // we don't know about, at least no features that would have ABI effects!
-                    // We skip this logic in rustdoc, where we want to allow all target features of
-                    // all targets, so we can't check their ABI compatibility and anyway we are not
-                    // generating code so "it's fine".
-                    if !tcx.sess.opts.actually_rustdoc {
-                        if abi_feature_constraints.incompatible.contains(&name.as_str()) {
-                            // For "neon" specifically, we emit an FCW instead of a hard error.
-                            // See <https://github.com/rust-lang/rust/issues/134375>.
-                            if tcx.sess.target.arch == "aarch64" && name.as_str() == "neon" {
-                                tcx.emit_node_span_lint(
-                                    AARCH64_SOFTFLOAT_NEON,
-                                    tcx.local_def_id_to_hir_id(did),
-                                    item.span(),
-                                    errors::Aarch64SoftfloatNeon,
-                                );
-                            } else {
-                                tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
-                                    span: item.span(),
-                                    feature: name.as_str(),
-                                    reason: "this feature is incompatible with the target ABI",
-                                });
-                            }
+        // Only allow target features whose feature gates have been enabled
+        // and which are permitted to be toggled.
+        if let Err(reason) = stability.toggle_allowed() {
+            tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
+                span: feature_span,
+                feature: feature_str,
+                reason,
+            });
+        } else if let Some(nightly_feature) = stability.requires_nightly()
+            && !rust_features.enabled(nightly_feature)
+        {
+            feature_err(
+                &tcx.sess,
+                nightly_feature,
+                feature_span,
+                format!("the target feature `{feature}` is currently unstable"),
+            )
+            .emit();
+        } else {
+            // Add this and the implied features.
+            for &name in tcx.implied_target_features(feature) {
+                // But ensure the ABI does not forbid enabling this.
+                // Here we do assume that the backend doesn't add even more implied features
+                // we don't know about, at least no features that would have ABI effects!
+                // We skip this logic in rustdoc, where we want to allow all target features of
+                // all targets, so we can't check their ABI compatibility and anyway we are not
+                // generating code so "it's fine".
+                if !tcx.sess.opts.actually_rustdoc {
+                    if abi_feature_constraints.incompatible.contains(&name.as_str()) {
+                        // For "neon" specifically, we emit an FCW instead of a hard error.
+                        // See <https://github.com/rust-lang/rust/issues/134375>.
+                        if tcx.sess.target.arch == "aarch64" && name.as_str() == "neon" {
+                            tcx.emit_node_span_lint(
+                                AARCH64_SOFTFLOAT_NEON,
+                                tcx.local_def_id_to_hir_id(did),
+                                feature_span,
+                                errors::Aarch64SoftfloatNeon,
+                            );
+                        } else {
+                            tcx.dcx().emit_err(errors::ForbiddenTargetFeatureAttr {
+                                span: feature_span,
+                                feature: name.as_str(),
+                                reason: "this feature is incompatible with the target ABI",
+                            });
                         }
                     }
-                    target_features.push(TargetFeature { name, implied: name != feature_sym })
                 }
+                target_features.push(TargetFeature { name, implied: name != feature })
             }
         }
     }