Unrolled build for #158313

Rollup merge of #158313 - obeis:move-check_target_feature, r=JonathanBrouwer

Move `check_target_feature` into the attribute parser

Updates #153101

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

r? @JonathanBrouwer

Author: rust-timer

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;
 
@@ -524,15 +524,6 @@ impl CombineAttributeParser for TargetFeatureParser {
         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)
-    }
-
     const ALLOWED_TARGETS: AllowedTargets<'_> = AllowedTargets::AllowList(&[
         Allow(Target::Fn),
         Allow(Target::Method(MethodKind::Inherent)),
@@ -544,6 +535,29 @@ impl CombineAttributeParser for TargetFeatureParser {
         Warn(Target::MacroDef),
         Warn(Target::MacroCall),
     ]);
+    const STABILITY: AttributeStability = AttributeStability::Stable;
+
+    fn extend(
+        cx: &mut AcceptContext<'_, '_>,
+        args: &ArgParser,
+    ) -> impl IntoIterator<Item = Self::Item> {
+        parse_tf_attribute(cx, args)
+    }
+
+    fn finalize_check(cx: &FinalizeContext<'_, '_>, attr_span: Span) {
+        // `#[target_feature]` is incompatible with lang item functions,
+        // except on WASM where calling target-feature functions is safe (see #84988).
+        if !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, kind, item_span: cx.target_span });
+            }
+        }
+    }
 }
 
 pub(crate) struct ForceTargetFeatureParser;

compiler/rustc_attr_parsing/src/attributes/link_attrs.rs

@@ -576,7 +576,7 @@ impl NoArgsAttributeParser for FfiPureParser {
     const STABILITY: AttributeStability = unstable!(ffi_pure);
     const CREATE: fn(Span) -> AttributeKind = AttributeKind::FfiPure;
 
-    fn finalize_check(attr_span: Span, cx: &FinalizeContext<'_, '_>) {
+    fn finalize_check(cx: &FinalizeContext<'_, '_>, attr_span: Span) {
         // `#[ffi_const]` functions cannot be `#[ffi_pure]`.
         if cx.all_attrs.iter().any(|a| a.word_is(sym::ffi_const)) {
             cx.emit_err(BothFfiConstAndPure { attr_span });

compiler/rustc_attr_parsing/src/attributes/mod.rs

@@ -154,7 +154,7 @@ pub(crate) trait SingleAttributeParser: 'static {
     /// reject incompatible combinations. `attr_span` is the span of this attribute.
     ///
     /// Defaults to a no-op.
-    fn finalize_check(_attr_span: Span, _cx: &FinalizeContext<'_, '_>) {}
+    fn finalize_check(_cx: &FinalizeContext<'_, '_>, _attr_span: Span) {}
 }
 
 /// Use in combination with [`SingleAttributeParser`].
@@ -187,7 +187,7 @@ impl<T: SingleAttributeParser> AttributeParser for Single<T> {
 
     fn finalize(self, cx: &FinalizeContext<'_, '_>) -> Option<AttributeKind> {
         let (kind, span) = self.1?;
-        T::finalize_check(span, cx);
+        T::finalize_check(cx, span);
         Some(kind)
     }
 }
@@ -275,7 +275,7 @@ pub(crate) trait NoArgsAttributeParser: 'static {
     /// `attr_span` is the span of this attribute.
     ///
     /// Defaults to a no-op.
-    fn finalize_check(_attr_span: Span, _cx: &FinalizeContext<'_, '_>) {}
+    fn finalize_check(_cx: &FinalizeContext<'_, '_>, _attr_span: Span) {}
 }
 
 pub(crate) struct WithoutArgs<T: NoArgsAttributeParser>(PhantomData<T>);
@@ -299,8 +299,8 @@ impl<T: NoArgsAttributeParser> SingleAttributeParser for WithoutArgs<T> {
         Some(T::CREATE(cx.attr_span))
     }
 
-    fn finalize_check(attr_span: Span, cx: &FinalizeContext<'_, '_>) {
-        T::finalize_check(attr_span, cx)
+    fn finalize_check(cx: &FinalizeContext<'_, '_>, attr_span: Span) {
+        T::finalize_check(cx, attr_span)
     }
 }
 
@@ -335,6 +335,14 @@ pub(crate) trait CombineAttributeParser: 'static {
         cx: &mut AcceptContext<'_, '_>,
         args: &ArgParser,
     ) -> impl IntoIterator<Item = Self::Item>;
+
+    /// Optional cross-attribute validation, run once during finalization after all
+    /// attributes on the item have been parsed. Has access to the sibling attributes via
+    /// [`FinalizeContext::all_attrs`], so it can reject incompatible combinations.
+    /// `attr_span` is the span of the first attribute that was encountered.
+    ///
+    /// Defaults to a no-op.
+    fn finalize_check(_cx: &FinalizeContext<'_, '_>, _attr_span: Span) {}
 }
 
 /// Use in combination with [`CombineAttributeParser`].
@@ -367,8 +375,9 @@ impl<T: CombineAttributeParser> AttributeParser for Combine<T> {
     const ALLOWED_TARGETS: AllowedTargets<'_> = T::ALLOWED_TARGETS;
     const SAFETY: AttributeSafety = T::SAFETY;
 
-    fn finalize(self, _cx: &FinalizeContext<'_, '_>) -> Option<AttributeKind> {
+    fn finalize(self, cx: &FinalizeContext<'_, '_>) -> Option<AttributeKind> {
         if let Some(first_span) = self.first_span {
+            T::finalize_check(cx, first_span);
             Some(T::CONVERT(self.items, first_span))
         } else {
             None

compiler/rustc_attr_parsing/src/session_diagnostics.rs

@@ -80,6 +80,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

@@ -204,9 +204,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);
             }
@@ -406,6 +403,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             AttributeKind::ShouldPanic { .. } => (),
             AttributeKind::Splat(..) => (),
             AttributeKind::Stability { .. } => (),
+            AttributeKind::TargetFeature { .. } => {}
             AttributeKind::TestRunner(..) => (),
             AttributeKind::ThreadLocal => (),
             AttributeKind::TypeLengthLimit { .. } => (),
@@ -799,37 +797,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

@@ -362,26 +362,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("duplicate diagnostic item in crate `{$crate_name}`: `{$name}`")]
 pub(crate) struct DuplicateDiagnosticItemInCrate {

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