Skip to content

Commit 9d0eaa2

Browse files
jieyouxujieyouxu
committed
check_attr: cover multi-segment attributes on specific check arms
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 ```rust,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]`). See <#128622>.
1 parent 9e5c9c1 commit 9d0eaa2

File tree

1 file changed

+73
-71
lines changed

1 file changed

+73
-71
lines changed

‎compiler/rustc_passes/src/check_attr.rs

+73-71
Original file line numberDiff line numberDiff line change
@@ -116,130 +116,130 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
116116
let attrs = self.tcx.hir().attrs(hir_id);
117117
for attr in attrs {
118118
match attr.path().as_slice() {
119-
[sym::diagnostic, sym::do_not_recommend] => {
119+
[sym::diagnostic, sym::do_not_recommend, ..] => {
120120
self.check_do_not_recommend(attr.span, hir_id, target)
121121
}
122-
[sym::diagnostic, sym::on_unimplemented] => {
122+
[sym::diagnostic, sym::on_unimplemented, ..] => {
123123
self.check_diagnostic_on_unimplemented(attr.span, hir_id, target)
124124
}
125-
[sym::inline] => self.check_inline(hir_id, attr, span, target),
126-
[sym::coverage] => self.check_coverage(attr, span, target),
127-
[sym::optimize] => self.check_optimize(hir_id, attr, target),
128-
[sym::non_exhaustive] => self.check_non_exhaustive(hir_id, attr, span, target),
129-
[sym::marker] => self.check_marker(hir_id, attr, span, target),
130-
[sym::target_feature] => {
125+
[sym::inline, ..] => self.check_inline(hir_id, attr, span, target),
126+
[sym::coverage, ..] => self.check_coverage(attr, span, target),
127+
[sym::optimize, ..] => self.check_optimize(hir_id, attr, target),
128+
[sym::non_exhaustive, ..] => self.check_non_exhaustive(hir_id, attr, span, target),
129+
[sym::marker, ..] => self.check_marker(hir_id, attr, span, target),
130+
[sym::target_feature, ..] => {
131131
self.check_target_feature(hir_id, attr, span, target, attrs)
132132
}
133-
[sym::thread_local] => self.check_thread_local(attr, span, target),
134-
[sym::track_caller] => {
133+
[sym::thread_local, ..] => self.check_thread_local(attr, span, target),
134+
[sym::track_caller, ..] => {
135135
self.check_track_caller(hir_id, attr.span, attrs, span, target)
136136
}
137-
[sym::doc] => self.check_doc_attrs(
137+
[sym::doc, ..] => self.check_doc_attrs(
138138
attr,
139139
hir_id,
140140
target,
141141
&mut specified_inline,
142142
&mut doc_aliases,
143143
),
144-
[sym::no_link] => self.check_no_link(hir_id, attr, span, target),
145-
[sym::export_name] => self.check_export_name(hir_id, attr, span, target),
146-
[sym::rustc_layout_scalar_valid_range_start]
147-
| [sym::rustc_layout_scalar_valid_range_end] => {
144+
[sym::no_link, ..] => self.check_no_link(hir_id, attr, span, target),
145+
[sym::export_name, ..] => self.check_export_name(hir_id, attr, span, target),
146+
[sym::rustc_layout_scalar_valid_range_start, ..]
147+
| [sym::rustc_layout_scalar_valid_range_end, ..] => {
148148
self.check_rustc_layout_scalar_valid_range(attr, span, target)
149149
}
150-
[sym::allow_internal_unstable] => {
150+
[sym::allow_internal_unstable, ..] => {
151151
self.check_allow_internal_unstable(hir_id, attr, span, target, attrs)
152152
}
153-
[sym::debugger_visualizer] => self.check_debugger_visualizer(attr, target),
154-
[sym::rustc_allow_const_fn_unstable] => {
153+
[sym::debugger_visualizer, ..] => self.check_debugger_visualizer(attr, target),
154+
[sym::rustc_allow_const_fn_unstable, ..] => {
155155
self.check_rustc_allow_const_fn_unstable(hir_id, attr, span, target)
156156
}
157-
[sym::rustc_std_internal_symbol] => {
157+
[sym::rustc_std_internal_symbol, ..] => {
158158
self.check_rustc_std_internal_symbol(attr, span, target)
159159
}
160-
[sym::naked] => self.check_naked(hir_id, attr, span, target, attrs),
161-
[sym::rustc_never_returns_null_ptr] => {
160+
[sym::naked, ..] => self.check_naked(hir_id, attr, span, target, attrs),
161+
[sym::rustc_never_returns_null_ptr, ..] => {
162162
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
163163
}
164-
[sym::rustc_legacy_const_generics] => {
164+
[sym::rustc_legacy_const_generics, ..] => {
165165
self.check_rustc_legacy_const_generics(hir_id, attr, span, target, item)
166166
}
167-
[sym::rustc_lint_query_instability] => {
167+
[sym::rustc_lint_query_instability, ..] => {
168168
self.check_rustc_lint_query_instability(hir_id, attr, span, target)
169169
}
170-
[sym::rustc_lint_diagnostics] => {
170+
[sym::rustc_lint_diagnostics, ..] => {
171171
self.check_rustc_lint_diagnostics(hir_id, attr, span, target)
172172
}
173-
[sym::rustc_lint_opt_ty] => self.check_rustc_lint_opt_ty(attr, span, target),
174-
[sym::rustc_lint_opt_deny_field_access] => {
173+
[sym::rustc_lint_opt_ty, ..] => self.check_rustc_lint_opt_ty(attr, span, target),
174+
[sym::rustc_lint_opt_deny_field_access, ..] => {
175175
self.check_rustc_lint_opt_deny_field_access(attr, span, target)
176176
}
177-
[sym::rustc_clean]
178-
| [sym::rustc_dirty]
179-
| [sym::rustc_if_this_changed]
180-
| [sym::rustc_then_this_would_need] => self.check_rustc_dirty_clean(attr),
181-
[sym::rustc_coinductive]
182-
| [sym::rustc_must_implement_one_of]
183-
| [sym::rustc_deny_explicit_impl]
184-
| [sym::const_trait] => self.check_must_be_applied_to_trait(attr, span, target),
185-
[sym::cmse_nonsecure_entry] => {
177+
[sym::rustc_clean, ..]
178+
| [sym::rustc_dirty, ..]
179+
| [sym::rustc_if_this_changed, ..]
180+
| [sym::rustc_then_this_would_need, ..] => self.check_rustc_dirty_clean(attr),
181+
[sym::rustc_coinductive, ..]
182+
| [sym::rustc_must_implement_one_of, ..]
183+
| [sym::rustc_deny_explicit_impl, ..]
184+
| [sym::const_trait, ..] => self.check_must_be_applied_to_trait(attr, span, target),
185+
[sym::cmse_nonsecure_entry, ..] => {
186186
self.check_cmse_nonsecure_entry(hir_id, attr, span, target)
187187
}
188-
[sym::collapse_debuginfo] => self.check_collapse_debuginfo(attr, span, target),
189-
[sym::must_not_suspend] => self.check_must_not_suspend(attr, span, target),
190-
[sym::must_use] => self.check_must_use(hir_id, attr, target),
191-
[sym::rustc_pass_by_value] => self.check_pass_by_value(attr, span, target),
192-
[sym::rustc_allow_incoherent_impl] => {
188+
[sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
189+
[sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
190+
[sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
191+
[sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
192+
[sym::rustc_allow_incoherent_impl, ..] => {
193193
self.check_allow_incoherent_impl(attr, span, target)
194194
}
195-
[sym::rustc_has_incoherent_inherent_impls] => {
195+
[sym::rustc_has_incoherent_inherent_impls, ..] => {
196196
self.check_has_incoherent_inherent_impls(attr, span, target)
197197
}
198-
[sym::ffi_pure] => self.check_ffi_pure(attr.span, attrs, target),
199-
[sym::ffi_const] => self.check_ffi_const(attr.span, target),
200-
[sym::rustc_const_unstable]
201-
| [sym::rustc_const_stable]
202-
| [sym::unstable]
203-
| [sym::stable]
204-
| [sym::rustc_allowed_through_unstable_modules]
205-
| [sym::rustc_promotable] => self.check_stability_promotable(attr, target),
206-
[sym::link_ordinal] => self.check_link_ordinal(attr, span, target),
207-
[sym::rustc_confusables] => self.check_confusables(attr, target),
208-
[sym::rustc_safe_intrinsic] => {
198+
[sym::ffi_pure, ..] => self.check_ffi_pure(attr.span, attrs, target),
199+
[sym::ffi_const, ..] => self.check_ffi_const(attr.span, target),
200+
[sym::rustc_const_unstable, ..]
201+
| [sym::rustc_const_stable, ..]
202+
| [sym::unstable, ..]
203+
| [sym::stable, ..]
204+
| [sym::rustc_allowed_through_unstable_modules, ..]
205+
| [sym::rustc_promotable, ..] => self.check_stability_promotable(attr, target),
206+
[sym::link_ordinal, ..] => self.check_link_ordinal(attr, span, target),
207+
[sym::rustc_confusables, ..] => self.check_confusables(attr, target),
208+
[sym::rustc_safe_intrinsic, ..] => {
209209
self.check_rustc_safe_intrinsic(hir_id, attr, span, target)
210210
}
211-
[sym::cold] => self.check_cold(hir_id, attr, span, target),
212-
[sym::link] => self.check_link(hir_id, attr, span, target),
213-
[sym::link_name] => self.check_link_name(hir_id, attr, span, target),
214-
[sym::link_section] => self.check_link_section(hir_id, attr, span, target),
215-
[sym::no_mangle] => self.check_no_mangle(hir_id, attr, span, target),
216-
[sym::deprecated] => self.check_deprecated(hir_id, attr, span, target),
217-
[sym::macro_use] | [sym::macro_escape] => {
211+
[sym::cold, ..] => self.check_cold(hir_id, attr, span, target),
212+
[sym::link, ..] => self.check_link(hir_id, attr, span, target),
213+
[sym::link_name, ..] => self.check_link_name(hir_id, attr, span, target),
214+
[sym::link_section, ..] => self.check_link_section(hir_id, attr, span, target),
215+
[sym::no_mangle, ..] => self.check_no_mangle(hir_id, attr, span, target),
216+
[sym::deprecated, ..] => self.check_deprecated(hir_id, attr, span, target),
217+
[sym::macro_use, ..] | [sym::macro_escape, ..] => {
218218
self.check_macro_use(hir_id, attr, target)
219219
}
220-
[sym::path] => self.check_generic_attr(hir_id, attr, target, Target::Mod),
221-
[sym::macro_export] => self.check_macro_export(hir_id, attr, target),
222-
[sym::ignore] | [sym::should_panic] => {
220+
[sym::path, ..] => self.check_generic_attr(hir_id, attr, target, Target::Mod),
221+
[sym::macro_export, ..] => self.check_macro_export(hir_id, attr, target),
222+
[sym::ignore, ..] | [sym::should_panic, ..] => {
223223
self.check_generic_attr(hir_id, attr, target, Target::Fn)
224224
}
225-
[sym::automatically_derived] => {
225+
[sym::automatically_derived, ..] => {
226226
self.check_generic_attr(hir_id, attr, target, Target::Impl)
227227
}
228-
[sym::no_implicit_prelude] => {
228+
[sym::no_implicit_prelude, ..] => {
229229
self.check_generic_attr(hir_id, attr, target, Target::Mod)
230230
}
231-
[sym::rustc_object_lifetime_default] => self.check_object_lifetime_default(hir_id),
232-
[sym::proc_macro] => {
231+
[sym::rustc_object_lifetime_default, ..] => self.check_object_lifetime_default(hir_id),
232+
[sym::proc_macro, ..] => {
233233
self.check_proc_macro(hir_id, target, ProcMacroKind::FunctionLike)
234234
}
235-
[sym::proc_macro_attribute] => {
235+
[sym::proc_macro_attribute, ..] => {
236236
self.check_proc_macro(hir_id, target, ProcMacroKind::Attribute);
237237
}
238-
[sym::proc_macro_derive] => {
238+
[sym::proc_macro_derive, ..] => {
239239
self.check_generic_attr(hir_id, attr, target, Target::Fn);
240240
self.check_proc_macro(hir_id, target, ProcMacroKind::Derive)
241241
}
242-
[sym::coroutine] => {
242+
[sym::coroutine, ..] => {
243243
self.check_coroutine(attr, target);
244244
}
245245
[
@@ -273,14 +273,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
273273
| sym::default_lib_allocator
274274
| sym::start
275275
| sym::custom_mir,
276+
..
276277
] => {}
277278
[name, ..] => {
278279
match BUILTIN_ATTRIBUTE_MAP.get(name) {
279280
// checked below
280281
Some(BuiltinAttribute { type_: AttributeType::CrateLevel, .. }) => {}
281282
Some(_) => {
282-
// FIXME: differentiate between unstable and internal attributes just like we do with features instead
283-
// of just accepting `rustc_` attributes by name. That should allow trimming the above list, too.
283+
// FIXME: differentiate between unstable and internal attributes just
284+
// like we do with features instead of just accepting `rustc_`
285+
// attributes by name. That should allow trimming the above list, too.
284286
if !name.as_str().starts_with("rustc_") {
285287
span_bug!(
286288
attr.span,

0 commit comments

Comments
 (0)