Unrolled build for rust-lang#128623

Rollup merge of rust-lang#128623 - jieyouxu:check-attr-ice, r=nnethercote

Do not fire unhandled attribute assertion on multi-segment `AttributeType::Normal` attributes with builtin attribute as first segment

### The Problem

In rust-lang#128581 I introduced an assertion to check that all builtin attributes are actually checked via
`CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes.
Unfortunately, the assertion had correctness problems as revealed in rust-lang#128622.

The match on attribute path segments looked like

```rs,ignore
// Normal handler
[sym::should_panic] => /* check is implemented */
// Fallback handler
[name, ..] => match BUILTIN_ATTRIBUTE_MAP.get(name) {
    // checked below
    Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
    Some(_) => {
        if !name.as_str().starts_with("rustc_") {
            span_bug!(
                attr.span,
                "builtin attribute {name:?} not handled by `CheckAttrVisitor`"
            )
        }
    }
    None => (),
}
```

However, it failed to account for edge cases such as an attribute whose:

1. path segments *starts* with a segment matching the name of a builtin attribute such as `should_panic`, and
2. the first segment's symbol does not start with `rustc_`, and
3. the matched builtin attribute is also of `AttributeType::Normal` attribute type upon registration with the builtin attribute map.

These conditions when all satisfied cause the span bug to be issued for e.g.
`#[should_panic::skip]` because the `[sym::should_panic]` arm is not matched (since it's
`[sym::should_panic, sym::skip]`).

### Proposed Solution

This PR tries to remedy that by adjusting all normal/specific handlers to not match exactly on a single segment, but instead match a prefix segment.

i.e.

```rs,ignore
// Normal handler, notice the `, ..` rest pattern
[sym::should_panic, ..] => /* check is implemented */
// Fallback handler
[name, ..] => match BUILTIN_ATTRIBUTE_MAP.get(name) {
    // checked below
    Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
    Some(_) => {
        if !name.as_str().starts_with("rustc_") {
            span_bug!(
                attr.span,
                "builtin attribute {name:?} not handled by `CheckAttrVisitor`"
            )
        }
    }
    None => (),
}
```

### Review Remarks

This PR contains 2 commits:

1. The first commit adds a regression test. This will ICE without the `CheckAttrVisitor` changes.
2. The second commit adjusts `CheckAttrVisitor` assertion logic. Once this commit is applied, the test should no longer ICE and produce the expected bless stderr.

Fixes rust-lang#128622.

r? ``@nnethercote`` (since you reviewed rust-lang#128581)

Author: rust-timer

compiler/rustc_passes/src/check_attr.rs

@@ -116,130 +116,130 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
         let attrs = self.tcx.hir().attrs(hir_id);
         for attr in attrs {
             match attr.path().as_slice() {
-                [sym::diagnostic, sym::do_not_recommend] => {
+                [sym::diagnostic, sym::do_not_recommend, ..] => {
                     self.check_do_not_recommend(attr.span, hir_id, target)
                 }
-                [sym::diagnostic, sym::on_unimplemented] => {
+                [sym::diagnostic, sym::on_unimplemented, ..] => {
                     self.check_diagnostic_on_unimplemented(attr.span, hir_id, target)
                 }
-                [sym::inline] => self.check_inline(hir_id, attr, span, target),
-                [sym::coverage] => self.check_coverage(attr, span, target),
-                [sym::optimize] => self.check_optimize(hir_id, attr, target),
-                [sym::non_exhaustive] => self.check_non_exhaustive(hir_id, attr, span, target),
-                [sym::marker] => self.check_marker(hir_id, attr, span, target),
-                [sym::target_feature] => {
+                [sym::inline, ..] => self.check_inline(hir_id, attr, span, target),
+                [sym::coverage, ..] => self.check_coverage(attr, span, target),
+                [sym::optimize, ..] => self.check_optimize(hir_id, attr, target),
+                [sym::non_exhaustive, ..] => self.check_non_exhaustive(hir_id, attr, span, target),
+                [sym::marker, ..] => self.check_marker(hir_id, attr, span, target),
+                [sym::target_feature, ..] => {
                     self.check_target_feature(hir_id, attr, span, target, attrs)
                 }
-                [sym::thread_local] => self.check_thread_local(attr, span, target),
-                [sym::track_caller] => {
+                [sym::thread_local, ..] => self.check_thread_local(attr, span, target),
+                [sym::track_caller, ..] => {
                     self.check_track_caller(hir_id, attr.span, attrs, span, target)
                 }
-                [sym::doc] => self.check_doc_attrs(
+                [sym::doc, ..] => self.check_doc_attrs(
                     attr,
                     hir_id,
                     target,
                     &mut specified_inline,
                     &mut doc_aliases,
                 ),
-                [sym::no_link] => self.check_no_link(hir_id, attr, span, target),
-                [sym::export_name] => self.check_export_name(hir_id, attr, span, target),
-                [sym::rustc_layout_scalar_valid_range_start]
-                | [sym::rustc_layout_scalar_valid_range_end] => {
+                [sym::no_link, ..] => self.check_no_link(hir_id, attr, span, target),
+                [sym::export_name, ..] => self.check_export_name(hir_id, attr, span, target),
+                [sym::rustc_layout_scalar_valid_range_start, ..]
+                | [sym::rustc_layout_scalar_valid_range_end, ..] => {
                     self.check_rustc_layout_scalar_valid_range(attr, span, target)
                 }
-                [sym::allow_internal_unstable] => {
+                [sym::allow_internal_unstable, ..] => {
                     self.check_allow_internal_unstable(hir_id, attr, span, target, attrs)
                 }
-                [sym::debugger_visualizer] => self.check_debugger_visualizer(attr, target),
-                [sym::rustc_allow_const_fn_unstable] => {
+                [sym::debugger_visualizer, ..] => self.check_debugger_visualizer(attr, target),
+                [sym::rustc_allow_const_fn_unstable, ..] => {
                     self.check_rustc_allow_const_fn_unstable(hir_id, attr, span, target)
                 }
-                [sym::rustc_std_internal_symbol] => {
+                [sym::rustc_std_internal_symbol, ..] => {
                     self.check_rustc_std_internal_symbol(attr, span, target)
                 }
-                [sym::naked] => self.check_naked(hir_id, attr, span, target, attrs),
-                [sym::rustc_never_returns_null_ptr] => {
+                [sym::naked, ..] => self.check_naked(hir_id, attr, span, target, attrs),
+                [sym::rustc_never_returns_null_ptr, ..] => {
                     self.check_applied_to_fn_or_method(hir_id, attr, span, target)
                 }
-                [sym::rustc_legacy_const_generics] => {
+                [sym::rustc_legacy_const_generics, ..] => {
                     self.check_rustc_legacy_const_generics(hir_id, attr, span, target, item)
                 }
-                [sym::rustc_lint_query_instability] => {
+                [sym::rustc_lint_query_instability, ..] => {
                     self.check_rustc_lint_query_instability(hir_id, attr, span, target)
                 }
-                [sym::rustc_lint_diagnostics] => {
+                [sym::rustc_lint_diagnostics, ..] => {
                     self.check_rustc_lint_diagnostics(hir_id, attr, span, target)
                 }
-                [sym::rustc_lint_opt_ty] => self.check_rustc_lint_opt_ty(attr, span, target),
-                [sym::rustc_lint_opt_deny_field_access] => {
+                [sym::rustc_lint_opt_ty, ..] => self.check_rustc_lint_opt_ty(attr, span, target),
+                [sym::rustc_lint_opt_deny_field_access, ..] => {
                     self.check_rustc_lint_opt_deny_field_access(attr, span, target)
                 }
-                [sym::rustc_clean]
-                | [sym::rustc_dirty]
-                | [sym::rustc_if_this_changed]
-                | [sym::rustc_then_this_would_need] => self.check_rustc_dirty_clean(attr),
-                [sym::rustc_coinductive]
-                | [sym::rustc_must_implement_one_of]
-                | [sym::rustc_deny_explicit_impl]
-                | [sym::const_trait] => self.check_must_be_applied_to_trait(attr, span, target),
-                [sym::cmse_nonsecure_entry] => {
+                [sym::rustc_clean, ..]
+                | [sym::rustc_dirty, ..]
+                | [sym::rustc_if_this_changed, ..]
+                | [sym::rustc_then_this_would_need, ..] => self.check_rustc_dirty_clean(attr),
+                [sym::rustc_coinductive, ..]
+                | [sym::rustc_must_implement_one_of, ..]
+                | [sym::rustc_deny_explicit_impl, ..]
+                | [sym::const_trait, ..] => self.check_must_be_applied_to_trait(attr, span, target),
+                [sym::cmse_nonsecure_entry, ..] => {
                     self.check_cmse_nonsecure_entry(hir_id, attr, span, target)
                 }
-                [sym::collapse_debuginfo] => self.check_collapse_debuginfo(attr, span, target),
-                [sym::must_not_suspend] => self.check_must_not_suspend(attr, span, target),
-                [sym::must_use] => self.check_must_use(hir_id, attr, target),
-                [sym::rustc_pass_by_value] => self.check_pass_by_value(attr, span, target),
-                [sym::rustc_allow_incoherent_impl] => {
+                [sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
+                [sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
+                [sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
+                [sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
+                [sym::rustc_allow_incoherent_impl, ..] => {
                     self.check_allow_incoherent_impl(attr, span, target)
                 }
-                [sym::rustc_has_incoherent_inherent_impls] => {
+                [sym::rustc_has_incoherent_inherent_impls, ..] => {
                     self.check_has_incoherent_inherent_impls(attr, span, target)
                 }
-                [sym::ffi_pure] => self.check_ffi_pure(attr.span, attrs, target),
-                [sym::ffi_const] => self.check_ffi_const(attr.span, target),
-                [sym::rustc_const_unstable]
-                | [sym::rustc_const_stable]
-                | [sym::unstable]
-                | [sym::stable]
-                | [sym::rustc_allowed_through_unstable_modules]
-                | [sym::rustc_promotable] => self.check_stability_promotable(attr, target),
-                [sym::link_ordinal] => self.check_link_ordinal(attr, span, target),
-                [sym::rustc_confusables] => self.check_confusables(attr, target),
-                [sym::rustc_safe_intrinsic] => {
+                [sym::ffi_pure, ..] => self.check_ffi_pure(attr.span, attrs, target),
+                [sym::ffi_const, ..] => self.check_ffi_const(attr.span, target),
+                [sym::rustc_const_unstable, ..]
+                | [sym::rustc_const_stable, ..]
+                | [sym::unstable, ..]
+                | [sym::stable, ..]
+                | [sym::rustc_allowed_through_unstable_modules, ..]
+                | [sym::rustc_promotable, ..] => self.check_stability_promotable(attr, target),
+                [sym::link_ordinal, ..] => self.check_link_ordinal(attr, span, target),
+                [sym::rustc_confusables, ..] => self.check_confusables(attr, target),
+                [sym::rustc_safe_intrinsic, ..] => {
                     self.check_rustc_safe_intrinsic(hir_id, attr, span, target)
                 }
-                [sym::cold] => self.check_cold(hir_id, attr, span, target),
-                [sym::link] => self.check_link(hir_id, attr, span, target),
-                [sym::link_name] => self.check_link_name(hir_id, attr, span, target),
-                [sym::link_section] => self.check_link_section(hir_id, attr, span, target),
-                [sym::no_mangle] => self.check_no_mangle(hir_id, attr, span, target),
-                [sym::deprecated] => self.check_deprecated(hir_id, attr, span, target),
-                [sym::macro_use] | [sym::macro_escape] => {
+                [sym::cold, ..] => self.check_cold(hir_id, attr, span, target),
+                [sym::link, ..] => self.check_link(hir_id, attr, span, target),
+                [sym::link_name, ..] => self.check_link_name(hir_id, attr, span, target),
+                [sym::link_section, ..] => self.check_link_section(hir_id, attr, span, target),
+                [sym::no_mangle, ..] => self.check_no_mangle(hir_id, attr, span, target),
+                [sym::deprecated, ..] => self.check_deprecated(hir_id, attr, span, target),
+                [sym::macro_use, ..] | [sym::macro_escape, ..] => {
                     self.check_macro_use(hir_id, attr, target)
                 }
-                [sym::path] => self.check_generic_attr(hir_id, attr, target, Target::Mod),
-                [sym::macro_export] => self.check_macro_export(hir_id, attr, target),
-                [sym::ignore] | [sym::should_panic] => {
+                [sym::path, ..] => self.check_generic_attr(hir_id, attr, target, Target::Mod),
+                [sym::macro_export, ..] => self.check_macro_export(hir_id, attr, target),
+                [sym::ignore, ..] | [sym::should_panic, ..] => {
                     self.check_generic_attr(hir_id, attr, target, Target::Fn)
                 }
-                [sym::automatically_derived] => {
+                [sym::automatically_derived, ..] => {
                     self.check_generic_attr(hir_id, attr, target, Target::Impl)
                 }
-                [sym::no_implicit_prelude] => {
+                [sym::no_implicit_prelude, ..] => {
                     self.check_generic_attr(hir_id, attr, target, Target::Mod)
                 }
-                [sym::rustc_object_lifetime_default] => self.check_object_lifetime_default(hir_id),
-                [sym::proc_macro] => {
+                [sym::rustc_object_lifetime_default, ..] => self.check_object_lifetime_default(hir_id),
+                [sym::proc_macro, ..] => {
                     self.check_proc_macro(hir_id, target, ProcMacroKind::FunctionLike)
                 }
-                [sym::proc_macro_attribute] => {
+                [sym::proc_macro_attribute, ..] => {
                     self.check_proc_macro(hir_id, target, ProcMacroKind::Attribute);
                 }
-                [sym::proc_macro_derive] => {
+                [sym::proc_macro_derive, ..] => {
                     self.check_generic_attr(hir_id, attr, target, Target::Fn);
                     self.check_proc_macro(hir_id, target, ProcMacroKind::Derive)
                 }
-                [sym::coroutine] => {
+                [sym::coroutine, ..] => {
                     self.check_coroutine(attr, target);
                 }
                 [
@@ -273,14 +273,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
                     | 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.
+                            // 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,

tests/ui/attributes/check-builtin-attr-ice.rs

@@ -0,0 +1,58 @@
+//! Regression test for #128622.
+//!
+//! PR #128581 introduced an assertion that all builtin attributes are actually checked via
+//! `CheckAttrVisitor` and aren't accidentally usable on completely unrelated HIR nodes.
+//! Unfortunately, the check had correctness problems.
+//!
+//! The match on attribute path segments looked like
+//!
+//! ```rs,ignore
+//! [sym::should_panic] => /* check is implemented */
+//! match BUILTIN_ATTRIBUTE_MAP.get(name) {
+//!     // checked below
+//!     Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
+//!     Some(_) => {
+//!         if !name.as_str().starts_with("rustc_") {
+//!             span_bug!(
+//!                 attr.span,
+//!                 "builtin attribute {name:?} not handled by `CheckAttrVisitor`"
+//!             )
+//!         }
+//!     }
+//!     None => (),
+//! }
+//! ```
+//!
+//! However, it failed to account for edge cases such as an attribute whose:
+//!
+//! 1. path segments *starts* with a builtin attribute such as `should_panic`
+//! 2. which does not start with `rustc_`, and
+//! 3. is also an `AttributeType::Normal` attribute upon registration with the builtin attribute map
+//!
+//! These conditions when all satisfied cause the span bug to be issued for e.g.
+//! `#[should_panic::skip]` because the `[sym::should_panic]` arm is not matched (since it's
+//! `[sym::should_panic, sym::skip]`).
+//!
+//! This test checks that the span bug is not fired for such cases.
+//!
+//! issue: rust-lang/rust#128622
+
+// Notably, `should_panic` is a `AttributeType::Normal` attribute that is checked separately.
+
+struct Foo {
+    #[should_panic::skip]
+    //~^ ERROR failed to resolve
+    pub field: u8,
+
+    #[should_panic::a::b::c]
+    //~^ ERROR failed to resolve
+    pub field2: u8,
+}
+
+fn foo() {}
+
+fn main() {
+    #[deny::skip]
+    //~^ ERROR failed to resolve
+    foo();
+}

tests/ui/attributes/check-builtin-attr-ice.stderr

@@ -0,0 +1,21 @@
+error[E0433]: failed to resolve: use of undeclared crate or module `should_panic`
+  --> $DIR/check-builtin-attr-ice.rs:43:7
+   |
+LL |     #[should_panic::skip]
+   |       ^^^^^^^^^^^^ use of undeclared crate or module `should_panic`
+
+error[E0433]: failed to resolve: use of undeclared crate or module `should_panic`
+  --> $DIR/check-builtin-attr-ice.rs:47:7
+   |
+LL |     #[should_panic::a::b::c]
+   |       ^^^^^^^^^^^^ use of undeclared crate or module `should_panic`
+
+error[E0433]: failed to resolve: use of undeclared crate or module `deny`
+  --> $DIR/check-builtin-attr-ice.rs:55:7
+   |
+LL |     #[deny::skip]
+   |       ^^^^ use of undeclared crate or module `deny`
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0433`.