Rollup merge of rust-lang#128581 - jieyouxu:checked-attr, r=nnethercote

Assert that all attributes are actually checked via `CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes

``@oli-obk's`` rust-lang#128444 with unreachable case removed to avoid that PR bitrotting away.
Based on rust-lang#128402.

This PR will make adding a new attribute ICE on any use of that attribute unless it gets a handler added in `rustc_passes::CheckAttrVisitor`.

r? ``@nnethercote`` (since you were the reviewer of the original PR)

Author: matthiaskrgr

compiler/rustc_feature/src/builtin_attrs.rs

@@ -703,21 +703,21 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
         EncodeCrossCrate::No, allocator_internals, experimental!(needs_allocator),
     ),
     gated!(
-        panic_runtime, Normal, template!(Word), WarnFollowing,
+        panic_runtime, CrateLevel, template!(Word), WarnFollowing,
         EncodeCrossCrate::No, experimental!(panic_runtime)
     ),
     gated!(
-        needs_panic_runtime, Normal, template!(Word), WarnFollowing,
+        needs_panic_runtime, CrateLevel, template!(Word), WarnFollowing,
         EncodeCrossCrate::No, experimental!(needs_panic_runtime)
     ),
     gated!(
-        compiler_builtins, Normal, template!(Word), WarnFollowing,
+        compiler_builtins, CrateLevel, template!(Word), WarnFollowing,
         EncodeCrossCrate::No,
         "the `#[compiler_builtins]` attribute is used to identify the `compiler_builtins` crate \
         which contains compiler-rt intrinsics and will never be stable",
     ),
     gated!(
-        profiler_runtime, Normal, template!(Word), WarnFollowing,
+        profiler_runtime, CrateLevel, template!(Word), WarnFollowing,
         EncodeCrossCrate::No,
         "the `#[profiler_runtime]` attribute is used to identify the `profiler_builtins` crate \
         which contains the profiler runtime and will never be stable",

compiler/rustc_passes/messages.ftl

@@ -100,6 +100,10 @@ passes_continue_labeled_block =
     .label = labeled blocks cannot be `continue`'d
     .block_label = labeled block the `continue` points to
 
+passes_coroutine_on_non_closure =
+    attribute should be applied to closures
+    .label = not a closure
+
 passes_coverage_not_fn_or_closure =
     attribute should be applied to a function definition or closure
     .label = not a function or closure

compiler/rustc_passes/src/check_attr.rs

@@ -20,13 +20,13 @@ use rustc_hir::{
     TraitItem, CRATE_HIR_ID, CRATE_OWNER_ID,
 };
 use rustc_macros::LintDiagnostic;
-use rustc_middle::bug;
 use rustc_middle::hir::nested_filter;
 use rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault;
 use rustc_middle::query::Providers;
 use rustc_middle::traits::ObligationCause;
 use rustc_middle::ty::error::{ExpectedFound, TypeError};
 use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::{bug, span_bug};
 use rustc_session::lint::builtin::{
     CONFLICTING_REPR_HINTS, INVALID_DOC_ATTRIBUTES, INVALID_MACRO_EXPORT_ARGUMENTS,
     UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, UNUSED_ATTRIBUTES,
@@ -239,7 +239,59 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                     self.check_generic_attr(hir_id, attr, target, Target::Fn);
                     self.check_proc_macro(hir_id, target, ProcMacroKind::Derive)
                 }
-                _ => {}
+                [sym::coroutine] => {
+                    self.check_coroutine(attr, target);
+                }
+                [
+                    // ok
+                    sym::allow
+                    | sym::expect
+                    | sym::warn
+                    | sym::deny
+                    | sym::forbid
+                    | sym::cfg
+                    // need to be fixed
+                    | sym::cfi_encoding // FIXME(cfi_encoding)
+                    | sym::may_dangle // FIXME(dropck_eyepatch)
+                    | sym::pointee // FIXME(derive_smart_pointer)
+                    | sym::linkage // FIXME(linkage)
+                    | sym::no_sanitize // FIXME(no_sanitize)
+                    | sym::omit_gdb_pretty_printer_section // FIXME(omit_gdb_pretty_printer_section)
+                    | sym::used // handled elsewhere to restrict to static items
+                    | sym::repr // handled elsewhere to restrict to type decls items
+                    | sym::instruction_set // broken on stable!!!
+                    | sym::windows_subsystem // broken on stable!!!
+                    | sym::patchable_function_entry // FIXME(patchable_function_entry)
+                    | sym::deprecated_safe // FIXME(deprecated_safe)
+                    // internal
+                    | sym::prelude_import
+                    | sym::panic_handler
+                    | sym::allow_internal_unsafe
+                    | sym::fundamental
+                    | sym::lang
+                    | sym::needs_allocator
+                    | sym::default_lib_allocator
+                    | sym::start
+                    | sym::custom_mir,
+                ] => {}
+                [name, ..] => {
+                    match BUILTIN_ATTRIBUTE_MAP.get(name) {
+                        // checked below
+                        Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
+                        Some(_) => {
+                            // FIXME: differentiate between unstable and internal attributes just like we do with features instead
+                            // of just accepting `rustc_` attributes by name. That should allow trimming the above list, too.
+                            if !name.as_str().starts_with("rustc_") {
+                                span_bug!(
+                                    attr.span,
+                                    "builtin attribute {name:?} not handled by `CheckAttrVisitor`"
+                                )
+                            }
+                        }
+                        None => (),
+                    }
+                }
+                [] => unreachable!(),
             }
 
             let builtin = attr.ident().and_then(|ident| BUILTIN_ATTRIBUTE_MAP.get(&ident.name));
@@ -376,6 +428,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
 
     /// Checks that `#[optimize(..)]` is applied to a function/closure/method,
     /// or to an impl block or module.
+    // FIXME(#128488): this should probably be elevated to an error?
     fn check_optimize(&self, hir_id: HirId, attr: &Attribute, target: Target) {
         match target {
             Target::Fn
@@ -2279,6 +2332,15 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
             self.abort.set(true);
         }
     }
+
+    fn check_coroutine(&self, attr: &Attribute, target: Target) {
+        match target {
+            Target::Closure => return,
+            _ => {
+                self.dcx().emit_err(errors::CoroutineOnNonClosure { span: attr.span });
+            }
+        }
+    }
 }
 
 impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {

compiler/rustc_passes/src/errors.rs

@@ -636,6 +636,13 @@ pub struct Confusables {
     pub attr_span: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(passes_coroutine_on_non_closure)]
+pub struct CoroutineOnNonClosure {
+    #[primary_span]
+    pub span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(passes_empty_confusables)]
 pub(crate) struct EmptyConfusables {

tests/ui/coroutine/invalid_attr_usage.rs

@@ -0,0 +1,11 @@
+//! The `coroutine` attribute is only allowed on closures.
+
+#![feature(coroutines)]
+
+#[coroutine]
+//~^ ERROR: attribute should be applied to closures
+struct Foo;
+
+#[coroutine]
+//~^ ERROR: attribute should be applied to closures
+fn main() {}

tests/ui/coroutine/invalid_attr_usage.stderr

@@ -0,0 +1,14 @@
+error: attribute should be applied to closures
+  --> $DIR/invalid_attr_usage.rs:5:1
+   |
+LL | #[coroutine]
+   | ^^^^^^^^^^^^
+
+error: attribute should be applied to closures
+  --> $DIR/invalid_attr_usage.rs:9:1
+   |
+LL | #[coroutine]
+   | ^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+