check_attr doesn't check every HIR node type that can carry attributes.

[eddyb]

The check_attr visitor here is missing several methods:

rust/compiler/rustc_passes/src/check_attr.rs

Lines 866 to 936 in e261649

	impl Visitor<'tcx> for CheckAttrVisitor<'tcx> {
	type Map = Map<'tcx>;
	fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
	NestedVisitorMap::OnlyBodies(self.tcx.hir())
	}
	fn visit_item(&mut self, item: &'tcx Item<'tcx>) {
	let target = Target::from_item(item);
	self.check_attributes(
	item.hir_id,
	item.attrs,
	&item.span,
	target,
	Some(ItemLike::Item(item)),
	);
	intravisit::walk_item(self, item)
	}
	fn visit_trait_item(&mut self, trait_item: &'tcx TraitItem<'tcx>) {
	let target = Target::from_trait_item(trait_item);
	self.check_attributes(trait_item.hir_id, &trait_item.attrs, &trait_item.span, target, None);
	intravisit::walk_trait_item(self, trait_item)
	}
	fn visit_foreign_item(&mut self, f_item: &'tcx ForeignItem<'tcx>) {
	let target = Target::from_foreign_item(f_item);
	self.check_attributes(
	f_item.hir_id,
	&f_item.attrs,
	&f_item.span,
	target,
	Some(ItemLike::ForeignItem(f_item)),
	);
	intravisit::walk_foreign_item(self, f_item)
	}
	fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) {
	let target = target_from_impl_item(self.tcx, impl_item);
	self.check_attributes(impl_item.hir_id, &impl_item.attrs, &impl_item.span, target, None);
	intravisit::walk_impl_item(self, impl_item)
	}
	fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt<'tcx>) {
	// When checking statements ignore expressions, they will be checked later.
	if let hir::StmtKind::Local(ref l) = stmt.kind {
	self.check_attributes(l.hir_id, &l.attrs, &stmt.span, Target::Statement, None);
	}
	intravisit::walk_stmt(self, stmt)
	}
	fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
	let target = match expr.kind {
	hir::ExprKind::Closure(..) => Target::Closure,
	_ => Target::Expression,
	};
	self.check_attributes(expr.hir_id, &expr.attrs, &expr.span, target, None);
	intravisit::walk_expr(self, expr)
	}
	fn visit_variant(
	&mut self,
	variant: &'tcx hir::Variant<'tcx>,
	generics: &'tcx hir::Generics<'tcx>,
	item_id: HirId,
	) {
	self.check_attributes(variant.id, variant.attrs, &variant.span, Target::Variant, None);
	intravisit::walk_variant(self, variant, generics, item_id)
	}
	}

A quick search (for attrs:) in the HIR definition finds these additional types that can contain attributes (they're also missing from the hir::Target enum, which should probably be renamed to AttrTarget but that's besides the point):

(click to see definition code snippets)

(summary: GenericParam, MacroDef, Arm, Param, StructField)

• GenericParam

  rust/compiler/rustc_hir/src/hir.rs

  Lines 425 to 428 in e261649

  	pub struct GenericParam<'hir> {
  	pub hir_id: HirId,
  	pub name: ParamName,
  	pub attrs: &'hir [Attribute],

• MacroDef (is this handled elsewhere?)

  rust/compiler/rustc_hir/src/hir.rs

  Lines 722 to 725 in e261649

  	pub struct MacroDef<'hir> {
  	pub ident: Ident,
  	pub vis: Visibility<'hir>,
  	pub attrs: &'hir [Attribute],

• Arm

  rust/compiler/rustc_hir/src/hir.rs

  Lines 1147 to 1151 in e261649

  	pub struct Arm<'hir> {
  	#[stable_hasher(ignore)]
  	pub hir_id: HirId,
  	pub span: Span,
  	pub attrs: &'hir [Attribute],

• Param

  rust/compiler/rustc_hir/src/hir.rs

  Lines 2183 to 2184 in e261649

  	pub struct Param<'hir> {
  	pub attrs: &'hir [Attribute],

• StructField

  rust/compiler/rustc_hir/src/hir.rs

  Lines 2390 to 2397 in e261649

  	pub struct StructField<'hir> {
  	pub span: Span,
  	#[stable_hasher(project(name))]
  	pub ident: Ident,
  	pub vis: Visibility<'hir>,
  	pub hir_id: HirId,
  	pub ty: &'hir Ty<'hir>,
  	pub attrs: &'hir [Attribute],

This isn't necessary a bug for some of them, as there is special validation elsewhere, e.g.:

rust/compiler/rustc_ast_passes/src/ast_validation.rs

Lines 377 to 378 in e261649

	"allow, cfg, cfg_attr, deny, \
	forbid, and warn are the only allowed built-in attributes in function parameters",

But it's not as uniform and future-proof as it could be. It might even be worth having a contextual attribute Target, and handling all attributes on visit_attr, or a hybrid solution that ICEs when visit_attr comes across an attribute not already handled.

And I did end up finding at least one bug, this compiles without any warnings (try on playground):

fn main() {
    match 5 {
        #[inline] _ => {}
    }
}

cc @varkor @davidtwco @petrochenkov

[rylev]

@rustbot claim

[eddyb]

After #80641 and #80920, what's left? cc @oli-obk

[oli-obk]

Looked over those PRs, yea, this is fixed now.