Move check_target_feature into the attribute parser

Use `TargetFeatureParser` to reject `#[target_feature]` on functions
marked as lang items (via `#[lang = "..."]` or `#[panic_handler]`),
replacing `check_target_feature` in `rustc_passes`.

Author: obeis

compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs

@@ -10,7 +10,7 @@ use crate::attributes::AttributeSafety;
 use crate::session_diagnostics::{
     EmptyExportName, NakedFunctionIncompatibleAttribute, NullOnExport, NullOnObjcClass,
     NullOnObjcSelector, ObjcClassExpectedStringLiteral, ObjcSelectorExpectedStringLiteral,
-    SanitizeInvalidStatic,
+    SanitizeInvalidStatic, TargetFeatureOnLangItem,
 };
 use crate::target_checking::Policy::AllowSilent;
 
@@ -513,26 +513,22 @@ fn parse_tf_attribute(
     features
 }
 
-pub(crate) struct TargetFeatureParser;
-
-impl CombineAttributeParser for TargetFeatureParser {
-    type Item = (Symbol, Span);
-    const PATH: &[Symbol] = &[sym::target_feature];
-    const CONVERT: ConvertFn<Self::Item> = |items, span| AttributeKind::TargetFeature {
-        features: items,
-        attr_span: span,
-        was_forced: false,
-    };
-    const TEMPLATE: AttributeTemplate = template!(List: &["enable = \"feat1, feat2\""]);
-    const STABILITY: AttributeStability = AttributeStability::Stable;
-
-    fn extend(
-        cx: &mut AcceptContext<'_, '_>,
-        args: &ArgParser,
-    ) -> impl IntoIterator<Item = Self::Item> {
-        parse_tf_attribute(cx, args)
-    }
+#[derive(Default)]
+pub(crate) struct TargetFeatureParser {
+    features: ThinVec<(Symbol, Span)>,
+    first_span: Option<Span>,
+}
 
+impl AttributeParser for TargetFeatureParser {
+    const ATTRIBUTES: AcceptMapping<Self> = &[(
+        &[sym::target_feature],
+        template!(List: &["enable = \"feat1, feat2\""]),
+        AttributeStability::Stable,
+        |this, cx, args| {
+            this.first_span.get_or_insert(cx.attr_span);
+            this.features.extend(parse_tf_attribute(cx, args));
+        },
+    )];
     const ALLOWED_TARGETS: AllowedTargets = AllowedTargets::AllowList(&[
         Allow(Target::Fn),
         Allow(Target::Method(MethodKind::Inherent)),
@@ -544,6 +540,38 @@ impl CombineAttributeParser for TargetFeatureParser {
         Warn(Target::MacroDef),
         Warn(Target::MacroCall),
     ]);
+
+    fn finalize(self, cx: &FinalizeContext<'_, '_>) -> Option<AttributeKind> {
+        let first_span = self.first_span?;
+
+        // `#[target_feature]` is incompatible with lang item functions,
+        // except on WASM where calling target-feature functions is safe (see #84988).
+        if matches!(
+            cx.target,
+            Target::Fn | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent)
+        ) && !cx.sess().target.is_like_wasm
+            && !cx.sess().opts.actually_rustdoc
+        {
+            // `#[panic_handler]` is checked first so it takes priority in the diagnostic.
+            let lang_kind = cx
+                .all_attrs
+                .iter()
+                .find_map(|a| [sym::panic_handler, sym::lang].into_iter().find(|&s| a.word_is(s)));
+            if let Some(kind) = lang_kind {
+                cx.emit_err(TargetFeatureOnLangItem {
+                    attr_span: first_span,
+                    kind,
+                    item_span: cx.target_span,
+                });
+            }
+        }
+
+        Some(AttributeKind::TargetFeature {
+            features: self.features,
+            attr_span: first_span,
+            was_forced: false,
+        })
+    }
 }
 
 pub(crate) struct ForceTargetFeatureParser;

compiler/rustc_attr_parsing/src/context.rs

@@ -172,9 +172,9 @@ attribute_parsers!(
         Combine<RustcDumpLayoutParser>,
         Combine<RustcMirParser>,
         Combine<RustcThenThisWouldNeedParser>,
-        Combine<TargetFeatureParser>,
         Combine<UnstableFeatureBoundParser>,
         Combine<UnstableRemovedParser>,
+        TargetFeatureParser,
         // tidy-alphabetical-end
 
         // tidy-alphabetical-start

compiler/rustc_attr_parsing/src/session_diagnostics.rs

@@ -73,6 +73,26 @@ pub(crate) struct DocAttributeNotAttribute {
     pub attribute: Symbol,
 }
 
+#[derive(Diagnostic)]
+#[diag(
+    "`#[target_feature]` cannot be applied to a {$kind ->
+        [panic_handler] `#[panic_handler]`
+        *[other] lang item
+    } function"
+)]
+pub(crate) struct TargetFeatureOnLangItem {
+    #[primary_span]
+    pub attr_span: Span,
+    pub kind: Symbol,
+    #[label(
+        "{$kind ->
+            [panic_handler] `#[panic_handler]`
+            *[other] lang item
+        } function is not allowed to have `#[target_feature]`"
+    )]
+    pub item_span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag("missing 'since'", code = E0542)]
 pub(crate) struct MissingSince {

compiler/rustc_passes/src/check_attr.rs

@@ -203,9 +203,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             AttributeKind::Deprecated { span: attr_span, .. } => {
                 self.check_deprecated(hir_id, *attr_span, target)
             }
-            AttributeKind::TargetFeature { attr_span, .. } => {
-                self.check_target_feature(hir_id, *attr_span, target, attrs)
-            }
             AttributeKind::RustcDumpObjectLifetimeDefaults => {
                 self.check_dump_object_lifetime_defaults(hir_id);
             }
@@ -405,6 +402,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             AttributeKind::ShouldPanic { .. } => (),
             AttributeKind::Splat(..) => (),
             AttributeKind::Stability { .. } => (),
+            AttributeKind::TargetFeature { .. } => {}
             AttributeKind::TestRunner(..) => (),
             AttributeKind::ThreadLocal => (),
             AttributeKind::TypeLengthLimit { .. } => (),
@@ -798,37 +796,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         }
     }
 
-    /// Checks if the `#[target_feature]` attribute on `item` is valid.
-    fn check_target_feature(
-        &self,
-        hir_id: HirId,
-        attr_span: Span,
-        target: Target,
-        attrs: &[Attribute],
-    ) {
-        match target {
-            Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent)
-            | Target::Fn => {
-                // `#[target_feature]` is not allowed in lang items.
-                if let Some(lang_item) = find_attr!(attrs, Lang(lang ) => lang)
-                    // Calling functions with `#[target_feature]` is
-                    // not unsafe on WASM, see #84988
-                    && !self.tcx.sess.target.is_like_wasm
-                    && !self.tcx.sess.opts.actually_rustdoc
-                {
-                    let sig = self.tcx.hir_node(hir_id).fn_sig().unwrap();
-
-                    self.dcx().emit_err(diagnostics::LangItemWithTargetFeature {
-                        attr_span,
-                        name: lang_item.name(),
-                        sig_span: sig.span,
-                    });
-                }
-            }
-            _ => {}
-        }
-    }
-
     fn check_doc_alias_value(&self, span: Span, hir_id: HirId, target: Target, alias: Symbol) {
         if let Some(location) = match target {
             Target::AssocTy => {

compiler/rustc_passes/src/diagnostics.rs

@@ -378,26 +378,6 @@ pub(crate) struct LangItemWithTrackCaller {
     pub sig_span: Span,
 }
 
-#[derive(Diagnostic)]
-#[diag(
-    "{$name ->
-        [panic_impl] `#[panic_handler]`
-        *[other] `{$name}` lang item
-    } function is not allowed to have `#[target_feature]`"
-)]
-pub(crate) struct LangItemWithTargetFeature {
-    #[primary_span]
-    pub attr_span: Span,
-    pub name: Symbol,
-    #[label(
-        "{$name ->
-            [panic_impl] `#[panic_handler]`
-            *[other] `{$name}` lang item
-        } function is not allowed to have `#[target_feature]`"
-    )]
-    pub sig_span: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag("`{$name}` lang item must be applied to a {$expected_target}", code = E0718)]
 pub(crate) struct LangItemOnIncorrectTarget {

tests/ui/lang-items/start_lang_item_with_target_feature.rs

@@ -18,7 +18,7 @@ pub trait Sized: MetaSized {}
 
 #[lang = "start"]
 #[target_feature(enable = "avx2")]
-//~^ ERROR `start` lang item function is not allowed to have `#[target_feature]`
+//~^ ERROR #[target_feature]` cannot be applied to a lang item function
 fn start<T>(_main: fn() -> T, _argc: isize, _argv: *const *const u8, _sigpipe: u8) -> isize {
     0
 }

tests/ui/lang-items/start_lang_item_with_target_feature.stderr

@@ -1,11 +1,13 @@
-error: `start` lang item function is not allowed to have `#[target_feature]`
+error: `#[target_feature]` cannot be applied to a lang item function
   --> $DIR/start_lang_item_with_target_feature.rs:20:1
    |
-LL | #[target_feature(enable = "avx2")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |   #[target_feature(enable = "avx2")]
+   |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 LL |
-LL | fn start<T>(_main: fn() -> T, _argc: isize, _argv: *const *const u8, _sigpipe: u8) -> isize {
-   | ------------------------------------------------------------------------------------------- `start` lang item function is not allowed to have `#[target_feature]`
+LL | / fn start<T>(_main: fn() -> T, _argc: isize, _argv: *const *const u8, _sigpipe: u8) -> isize {
+LL | |     0
+LL | | }
+   | |_- lang item function is not allowed to have `#[target_feature]`
 
 error: aborting due to 1 previous error
 

tests/ui/panic-handler/panic-handler-with-target-feature.rs

@@ -8,7 +8,7 @@ use core::panic::PanicInfo;
 
 #[panic_handler]
 #[target_feature(enable = "avx2")]
-//~^ ERROR `#[panic_handler]` function is not allowed to have `#[target_feature]`
+//~^ ERROR `#[target_feature]` cannot be applied to a `#[panic_handler]` function
 fn panic(info: &PanicInfo) -> ! {
     unimplemented!();
 }

tests/ui/panic-handler/panic-handler-with-target-feature.stderr

@@ -1,11 +1,13 @@
-error: `#[panic_handler]` function is not allowed to have `#[target_feature]`
+error: `#[target_feature]` cannot be applied to a `#[panic_handler]` function
   --> $DIR/panic-handler-with-target-feature.rs:10:1
    |
-LL | #[target_feature(enable = "avx2")]
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |   #[target_feature(enable = "avx2")]
+   |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 LL |
-LL | fn panic(info: &PanicInfo) -> ! {
-   | ------------------------------- `#[panic_handler]` function is not allowed to have `#[target_feature]`
+LL | / fn panic(info: &PanicInfo) -> ! {
+LL | |     unimplemented!();
+LL | | }
+   | |_- `#[panic_handler]` function is not allowed to have `#[target_feature]`
 
 error: aborting due to 1 previous error