mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-22 14:55:26 +00:00
20480075bd
Do not fire unhandled attribute assertion on multi-segment `AttributeType::Normal` attributes with builtin attribute as first segment ### The Problem In #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 #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 #128622. r? ``@nnethercote`` (since you reviewed #128581) |
||
---|---|---|
.. | ||
rustc | ||
rustc_abi | ||
rustc_arena | ||
rustc_ast | ||
rustc_ast_ir | ||
rustc_ast_lowering | ||
rustc_ast_passes | ||
rustc_ast_pretty | ||
rustc_attr | ||
rustc_baked_icu_data | ||
rustc_borrowck | ||
rustc_builtin_macros | ||
rustc_codegen_cranelift | ||
rustc_codegen_gcc | ||
rustc_codegen_llvm | ||
rustc_codegen_ssa | ||
rustc_const_eval | ||
rustc_data_structures | ||
rustc_driver | ||
rustc_driver_impl | ||
rustc_error_codes | ||
rustc_error_messages | ||
rustc_errors | ||
rustc_expand | ||
rustc_feature | ||
rustc_fluent_macro | ||
rustc_fs_util | ||
rustc_graphviz | ||
rustc_hir | ||
rustc_hir_analysis | ||
rustc_hir_pretty | ||
rustc_hir_typeck | ||
rustc_incremental | ||
rustc_index | ||
rustc_index_macros | ||
rustc_infer | ||
rustc_interface | ||
rustc_lexer | ||
rustc_lint | ||
rustc_lint_defs | ||
rustc_llvm | ||
rustc_log | ||
rustc_macros | ||
rustc_metadata | ||
rustc_middle | ||
rustc_mir_build | ||
rustc_mir_dataflow | ||
rustc_mir_transform | ||
rustc_monomorphize | ||
rustc_next_trait_solver | ||
rustc_parse | ||
rustc_parse_format | ||
rustc_passes | ||
rustc_pattern_analysis | ||
rustc_privacy | ||
rustc_query_impl | ||
rustc_query_system | ||
rustc_resolve | ||
rustc_sanitizers | ||
rustc_serialize | ||
rustc_session | ||
rustc_smir | ||
rustc_span | ||
rustc_symbol_mangling | ||
rustc_target | ||
rustc_trait_selection | ||
rustc_traits | ||
rustc_transmute | ||
rustc_ty_utils | ||
rustc_type_ir | ||
rustc_type_ir_macros | ||
stable_mir |