From 67a08b5deba5a6fdf5fd4acfc095851f79172e59 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 30 Jul 2024 15:02:40 +0000 Subject: [PATCH] Attribute checking simplifications remove an unused boolean and then merge two big matches into one --- compiler/rustc_passes/src/check_attr.rs | 546 ++++++++---------------- 1 file changed, 179 insertions(+), 367 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index a58c57041f830..07c0a116bfeaf 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -207,37 +207,34 @@ impl<'tcx> CheckAttrVisitor<'tcx> { [sym::rustc_safe_intrinsic] => { self.check_rustc_safe_intrinsic(hir_id, attr, span, target) } - _ => true, - }; - - // lint-only checks - match attr.name_or_empty() { - 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::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] => { 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) } @@ -297,7 +294,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } /// Checks if `#[diagnostic::do_not_recommend]` is applied on a trait impl. - fn check_do_not_recommend(&self, attr_span: Span, hir_id: HirId, target: Target) -> bool { + fn check_do_not_recommend(&self, attr_span: Span, hir_id: HirId, target: Target) { if !matches!(target, Target::Impl) { self.tcx.emit_node_span_lint( UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, @@ -306,16 +303,10 @@ impl<'tcx> CheckAttrVisitor<'tcx> { errors::IncorrectDoNotRecommendLocation, ); } - true } /// Checks if `#[diagnostic::on_unimplemented]` is applied to a trait definition - fn check_diagnostic_on_unimplemented( - &self, - attr_span: Span, - hir_id: HirId, - target: Target, - ) -> bool { + fn check_diagnostic_on_unimplemented(&self, attr_span: Span, hir_id: HirId, target: Target) { if !matches!(target, Target::Trait) { self.tcx.emit_node_span_lint( UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, @@ -324,68 +315,60 @@ impl<'tcx> CheckAttrVisitor<'tcx> { DiagnosticOnUnimplementedOnlyForTraits, ); } - true } - /// Checks if an `#[inline]` is applied to a function or a closure. Returns `true` if valid. - fn check_inline(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool { + /// Checks if an `#[inline]` is applied to a function or a closure. + fn check_inline(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { match target { Target::Fn | Target::Closure - | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true, + | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {} Target::Method(MethodKind::Trait { body: false }) | Target::ForeignFn => { self.tcx.emit_node_span_lint( UNUSED_ATTRIBUTES, hir_id, attr.span, errors::IgnoredInlineAttrFnProto, - ); - true + ) } // FIXME(#65833): We permit associated consts to have an `#[inline]` attribute with // just a lint, because we previously erroneously allowed it and some crates used it // accidentally, to be compatible with crates depending on them, we can't throw an // error here. - Target::AssocConst => { - self.tcx.emit_node_span_lint( - UNUSED_ATTRIBUTES, - hir_id, - attr.span, - errors::IgnoredInlineAttrConstants, - ); - true - } + Target::AssocConst => self.tcx.emit_node_span_lint( + UNUSED_ATTRIBUTES, + hir_id, + attr.span, + errors::IgnoredInlineAttrConstants, + ), // FIXME(#80564): Same for fields, arms, and macro defs Target::Field | Target::Arm | Target::MacroDef => { - self.inline_attr_str_error_with_macro_def(hir_id, attr, "inline"); - true + self.inline_attr_str_error_with_macro_def(hir_id, attr, "inline") } _ => { self.dcx().emit_err(errors::InlineNotFnOrClosure { attr_span: attr.span, defn_span: span, }); - false } } } /// Checks that `#[coverage(..)]` is applied to a function/closure/method, /// or to an impl block or module. - fn check_coverage(&self, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_coverage(&self, attr: &Attribute, span: Span, target: Target) { match target { Target::Fn | Target::Closure | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) | Target::Impl - | Target::Mod => true, + | Target::Mod => {} _ => { self.dcx().emit_err(errors::CoverageNotFnOrClosure { attr_span: attr.span, defn_span: span, }); - false } } } @@ -418,7 +401,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { span: Span, target: Target, attrs: &[Attribute], - ) -> bool { + ) { // many attributes don't make sense in combination with #[naked]. // Notable attributes that are incompatible with `#[naked]` are: // @@ -468,19 +451,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: other_attr.name_or_empty(), }); - return false; + return; } } - - true } // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[naked]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { - self.inline_attr_str_error_with_macro_def(hir_id, attr, "naked"); - true + self.inline_attr_str_error_with_macro_def(hir_id, attr, "naked") } _ => { self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { @@ -488,7 +468,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { defn_span: span, on_crate: hir_id == CRATE_HIR_ID, }); - false } } } @@ -500,17 +479,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: &Attribute, span: Span, target: Target, - ) -> bool { + ) { match target { Target::Fn - | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true, + | Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {} _ => { self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { attr_span: attr.span, defn_span: span, on_crate: hir_id == CRATE_HIR_ID, }); - false } } } @@ -536,19 +514,18 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } /// Checks if `#[collapse_debuginfo]` is applied to a macro. - fn check_collapse_debuginfo(&self, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_collapse_debuginfo(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::MacroDef => true, + Target::MacroDef => {} _ => { self.tcx .dcx() .emit_err(errors::CollapseDebuginfo { attr_span: attr.span, defn_span: span }); - false } } } - /// Checks if a `#[track_caller]` is applied to a function. Returns `true` if valid. + /// Checks if a `#[track_caller]` is applied to a function. fn check_track_caller( &self, hir_id: HirId, @@ -556,7 +533,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attrs: &[Attribute], span: Span, target: Target, - ) -> bool { + ) { match target { Target::Fn => { // `#[track_caller]` is not valid on weak lang items because they are called via @@ -572,12 +549,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { name: lang_item, sig_span: sig.span, }); - false - } else { - true } } - Target::Method(..) | Target::ForeignFn | Target::Closure => true, + Target::Method(..) | Target::ForeignFn | Target::Closure => {} // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[track_caller]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible @@ -586,7 +560,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { for attr in attrs { self.inline_attr_str_error_with_macro_def(hir_id, attr, "track_caller"); } - true } _ => { self.dcx().emit_err(errors::TrackedCallerWrongLocation { @@ -594,62 +567,51 @@ impl<'tcx> CheckAttrVisitor<'tcx> { defn_span: span, on_crate: hir_id == CRATE_HIR_ID, }); - false } } } - /// Checks if the `#[non_exhaustive]` attribute on an `item` is valid. Returns `true` if valid. - fn check_non_exhaustive( - &self, - hir_id: HirId, - attr: &Attribute, - span: Span, - target: Target, - ) -> bool { + /// Checks if the `#[non_exhaustive]` attribute on an `item` is valid. + fn check_non_exhaustive(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { match target { - Target::Struct | Target::Enum | Target::Variant => true, + Target::Struct | Target::Enum | Target::Variant => {} // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[non_exhaustive]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { self.inline_attr_str_error_with_macro_def(hir_id, attr, "non_exhaustive"); - true } _ => { self.dcx().emit_err(errors::NonExhaustiveWrongLocation { attr_span: attr.span, defn_span: span, }); - false } } } - /// Checks if the `#[marker]` attribute on an `item` is valid. Returns `true` if valid. - fn check_marker(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool { + /// Checks if the `#[marker]` attribute on an `item` is valid. + fn check_marker(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { match target { - Target::Trait => true, + Target::Trait => {} // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[marker]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { self.inline_attr_str_error_with_macro_def(hir_id, attr, "marker"); - true } _ => { self.dcx().emit_err(errors::AttrShouldBeAppliedToTrait { attr_span: attr.span, defn_span: span, }); - false } } } - /// Checks if the `#[target_feature]` attribute on `item` is valid. Returns `true` if valid. + /// Checks if the `#[target_feature]` attribute on `item` is valid. fn check_target_feature( &self, hir_id: HirId, @@ -657,7 +619,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { span: Span, target: Target, attrs: &[Attribute], - ) -> bool { + ) { match target { Target::Fn => { // `#[target_feature]` is not allowed in lang items. @@ -674,12 +636,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { name: lang_item, sig_span: sig.span, }); - false - } else { - true } } - Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => true, + Target::Method(MethodKind::Trait { body: true } | MethodKind::Inherent) => {} // FIXME: #[target_feature] was previously erroneously allowed on statements and some // crates used this, so only emit a warning. Target::Statement => { @@ -689,7 +648,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr.span, errors::TargetFeatureOnStatement, ); - true } // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[target_feature]` attribute with just a lint, because we previously @@ -697,7 +655,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { self.inline_attr_str_error_with_macro_def(hir_id, attr, "target_feature"); - true } _ => { self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { @@ -705,21 +662,19 @@ impl<'tcx> CheckAttrVisitor<'tcx> { defn_span: span, on_crate: hir_id == CRATE_HIR_ID, }); - false } } } - /// Checks if the `#[thread_local]` attribute on `item` is valid. Returns `true` if valid. - fn check_thread_local(&self, attr: &Attribute, span: Span, target: Target) -> bool { + /// Checks if the `#[thread_local]` attribute on `item` is valid. + fn check_thread_local(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::ForeignStatic | Target::Static => true, + Target::ForeignStatic | Target::Static => {} _ => { self.dcx().emit_err(errors::AttrShouldBeAppliedToStatic { attr_span: attr.span, defn_span: span, }); - false } } } @@ -736,14 +691,14 @@ impl<'tcx> CheckAttrVisitor<'tcx> { target: Target, is_list: bool, aliases: &mut FxHashMap, - ) -> bool { + ) { let tcx = self.tcx; let span = meta.name_value_literal_span().unwrap_or_else(|| meta.span()); let attr_str = &format!("`#[doc(alias{})]`", if is_list { "(\"...\")" } else { " = \"...\"" }); if doc_alias == kw::Empty { tcx.dcx().emit_err(errors::DocAliasEmpty { span, attr_str }); - return false; + return; } let doc_alias_str = doc_alias.as_str(); @@ -752,11 +707,11 @@ impl<'tcx> CheckAttrVisitor<'tcx> { .find(|&c| c == '"' || c == '\'' || (c.is_whitespace() && c != ' ')) { tcx.dcx().emit_err(errors::DocAliasBadChar { span, attr_str, char_: c }); - return false; + return; } if doc_alias_str.starts_with(' ') || doc_alias_str.ends_with(' ') { tcx.dcx().emit_err(errors::DocAliasStartEnd { span, attr_str }); - return false; + return; } let span = meta.span(); @@ -781,7 +736,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } // we check the validity of params elsewhere - Target::Param => return false, + Target::Param => return, Target::Expression | Target::Statement | Target::Arm @@ -814,12 +769,12 @@ impl<'tcx> CheckAttrVisitor<'tcx> { | Target::ExprField => None, } { tcx.dcx().emit_err(errors::DocAliasBadLocation { span, attr_str, location }); - return false; + return; } let item_name = self.tcx.hir().name(hir_id); if item_name == doc_alias { tcx.dcx().emit_err(errors::DocAliasNotAnAlias { span, attr_str }); - return false; + return; } if let Err(entry) = aliases.try_insert(doc_alias_str.to_owned(), span) { self.tcx.emit_node_span_lint( @@ -829,7 +784,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { errors::DocAliasDuplicated { first_defn: *entry.entry.get() }, ); } - true } fn check_doc_alias( @@ -838,46 +792,39 @@ impl<'tcx> CheckAttrVisitor<'tcx> { hir_id: HirId, target: Target, aliases: &mut FxHashMap, - ) -> bool { + ) { if let Some(values) = meta.meta_item_list() { - let mut errors = 0; for v in values { match v.lit() { Some(l) => match l.kind { LitKind::Str(s, _) => { - if !self.check_doc_alias_value(v, s, hir_id, target, true, aliases) { - errors += 1; - } + self.check_doc_alias_value(v, s, hir_id, target, true, aliases); } _ => { self.tcx .dcx() .emit_err(errors::DocAliasNotStringLiteral { span: v.span() }); - errors += 1; } }, None => { self.tcx .dcx() .emit_err(errors::DocAliasNotStringLiteral { span: v.span() }); - errors += 1; } } } - errors == 0 } else if let Some(doc_alias) = meta.value_str() { self.check_doc_alias_value(meta, doc_alias, hir_id, target, false, aliases) } else { self.dcx().emit_err(errors::DocAliasMalformed { span: meta.span() }); - false } } - fn check_doc_keyword(&self, meta: &NestedMetaItem, hir_id: HirId) -> bool { + fn check_doc_keyword(&self, meta: &NestedMetaItem, hir_id: HirId) { let doc_keyword = meta.value_str().unwrap_or(kw::Empty); if doc_keyword == kw::Empty { self.doc_attr_str_error(meta, "keyword"); - return false; + return; } let item_kind = match self.tcx.hir_node(hir_id) { hir::Node::Item(item) => Some(&item.kind), @@ -887,12 +834,12 @@ impl<'tcx> CheckAttrVisitor<'tcx> { Some(ItemKind::Mod(module)) => { if !module.item_ids.is_empty() { self.dcx().emit_err(errors::DocKeywordEmptyMod { span: meta.span() }); - return false; + return; } } _ => { self.dcx().emit_err(errors::DocKeywordNotMod { span: meta.span() }); - return false; + return; } } if !rustc_lexer::is_ident(doc_keyword.as_str()) { @@ -900,12 +847,10 @@ impl<'tcx> CheckAttrVisitor<'tcx> { span: meta.name_value_literal_span().unwrap_or_else(|| meta.span()), doc_keyword, }); - return false; } - true } - fn check_doc_fake_variadic(&self, meta: &NestedMetaItem, hir_id: HirId) -> bool { + fn check_doc_fake_variadic(&self, meta: &NestedMetaItem, hir_id: HirId) { let item_kind = match self.tcx.hir_node(hir_id) { hir::Node::Item(item) => Some(&item.kind), _ => None, @@ -920,18 +865,15 @@ impl<'tcx> CheckAttrVisitor<'tcx> { }; if !is_valid { self.dcx().emit_err(errors::DocFakeVariadicNotValid { span: meta.span() }); - return false; } } _ => { self.dcx().emit_err(errors::DocKeywordOnlyImpl { span: meta.span() }); - return false; } } - true } - /// Checks `#[doc(inline)]`/`#[doc(no_inline)]` attributes. Returns `true` if valid. + /// Checks `#[doc(inline)]`/`#[doc(no_inline)]` attributes. /// /// A doc inlining attribute is invalid if it is applied to a non-`use` item, or /// if there are conflicting attributes for one item. @@ -947,7 +889,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { hir_id: HirId, target: Target, specified_inline: &mut Option<(bool, Span)>, - ) -> bool { + ) { match target { Target::Use | Target::ExternCrate => { let do_inline = meta.name_or_empty() == sym::inline; @@ -960,12 +902,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { fluent::passes_doc_inline_conflict_second, ); self.dcx().emit_err(errors::DocKeywordConflict { spans }); - return false; } - true } else { *specified_inline = Some((do_inline, meta.span())); - true } } _ => { @@ -979,7 +918,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { .then(|| self.tcx.hir().span(hir_id)), }, ); - false } } } @@ -990,7 +928,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { meta: &NestedMetaItem, hir_id: HirId, target: Target, - ) -> bool { + ) { if target != Target::ExternCrate { self.tcx.emit_node_span_lint( INVALID_DOC_ATTRIBUTES, @@ -1002,7 +940,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { .then(|| self.tcx.hir().span(hir_id)), }, ); - return false; + return; } if self.tcx.extern_mod_stmt_cnum(hir_id.owner).is_none() { @@ -1016,10 +954,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { .then(|| self.tcx.hir().span(hir_id)), }, ); - return false; } - - true } /// Checks that an attribute is *not* used at the crate level. Returns `true` if valid. @@ -1064,8 +999,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { /// Checks that `doc(test(...))` attribute contains only valid attributes. Returns `true` if /// valid. - fn check_test_attr(&self, meta: &NestedMetaItem, hir_id: HirId) -> bool { - let mut is_valid = true; + fn check_test_attr(&self, meta: &NestedMetaItem, hir_id: HirId) { if let Some(metas) = meta.meta_item_list() { for i_meta in metas { match (i_meta.name_or_empty(), i_meta.meta_item()) { @@ -1079,7 +1013,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { path: rustc_ast_pretty::pprust::path_to_string(&m.path), }, ); - is_valid = false; } (_, None) => { self.tcx.emit_node_span_lint( @@ -1088,7 +1021,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { i_meta.span(), errors::DocTestLiteral, ); - is_valid = false; } } } @@ -1099,28 +1031,23 @@ impl<'tcx> CheckAttrVisitor<'tcx> { meta.span(), errors::DocTestTakesList, ); - is_valid = false; } - is_valid } /// Check that the `#![doc(cfg_hide(...))]` attribute only contains a list of attributes. - /// Returns `true` if valid. - fn check_doc_cfg_hide(&self, meta: &NestedMetaItem, hir_id: HirId) -> bool { - if meta.meta_item_list().is_some() { - true - } else { + /// + fn check_doc_cfg_hide(&self, meta: &NestedMetaItem, hir_id: HirId) { + if meta.meta_item_list().is_none() { self.tcx.emit_node_span_lint( INVALID_DOC_ATTRIBUTES, hir_id, meta.span(), errors::DocCfgHideTakesList, ); - false } } - /// Runs various checks on `#[doc]` attributes. Returns `true` if valid. + /// Runs various checks on `#[doc]` attributes. /// /// `specified_inline` should be initialized to `None` and kept for the scope /// of one item. Read the documentation of [`check_doc_inline`] for more information. @@ -1134,34 +1061,35 @@ impl<'tcx> CheckAttrVisitor<'tcx> { target: Target, specified_inline: &mut Option<(bool, Span)>, aliases: &mut FxHashMap, - ) -> bool { - let mut is_valid = true; - + ) { if let Some(mi) = attr.meta() && let Some(list) = mi.meta_item_list() { for meta in list { if let Some(i_meta) = meta.meta_item() { match i_meta.name_or_empty() { - sym::alias - if !self.check_attr_not_crate_level(meta, hir_id, "alias") - || !self.check_doc_alias(meta, hir_id, target, aliases) => - { - is_valid = false + sym::alias => { + if self.check_attr_not_crate_level(meta, hir_id, "alias") { + self.check_doc_alias(meta, hir_id, target, aliases); + } + } + + sym::keyword => { + if self.check_attr_not_crate_level(meta, hir_id, "keyword") { + self.check_doc_keyword(meta, hir_id); + } } - sym::keyword - if !self.check_attr_not_crate_level(meta, hir_id, "keyword") - || !self.check_doc_keyword(meta, hir_id) => - { - is_valid = false + sym::fake_variadic => { + if self.check_attr_not_crate_level(meta, hir_id, "fake_variadic") { + self.check_doc_fake_variadic(meta, hir_id); + } } - sym::fake_variadic - if !self.check_attr_not_crate_level(meta, hir_id, "fake_variadic") - || !self.check_doc_fake_variadic(meta, hir_id) => - { - is_valid = false + sym::test => { + if self.check_attr_crate_level(attr, meta, hir_id) { + self.check_test_attr(meta, hir_id); + } } sym::html_favicon_url @@ -1169,62 +1097,36 @@ impl<'tcx> CheckAttrVisitor<'tcx> { | sym::html_playground_url | sym::issue_tracker_base_url | sym::html_root_url - | sym::html_no_source - | sym::test - | sym::rust_logo - if !self.check_attr_crate_level(attr, meta, hir_id) => - { - is_valid = false; + | sym::html_no_source => { + self.check_attr_crate_level(attr, meta, hir_id); } - sym::cfg_hide - if !self.check_attr_crate_level(attr, meta, hir_id) - || !self.check_doc_cfg_hide(meta, hir_id) => - { - is_valid = false; + sym::cfg_hide => { + if self.check_attr_crate_level(attr, meta, hir_id) { + self.check_doc_cfg_hide(meta, hir_id); + } } - sym::inline | sym::no_inline - if !self.check_doc_inline( - attr, - meta, - hir_id, - target, - specified_inline, - ) => - { - is_valid = false; + sym::inline | sym::no_inline => { + self.check_doc_inline(attr, meta, hir_id, target, specified_inline) } - sym::masked if !self.check_doc_masked(attr, meta, hir_id, target) => { - is_valid = false; - } + sym::masked => self.check_doc_masked(attr, meta, hir_id, target), // no_default_passes: deprecated // passes: deprecated // plugins: removed, but rustdoc warns about it itself - sym::alias - | sym::cfg - | sym::cfg_hide + sym::cfg | sym::hidden - | sym::html_favicon_url - | sym::html_logo_url - | sym::html_no_source - | sym::html_playground_url - | sym::html_root_url - | sym::inline - | sym::issue_tracker_base_url - | sym::keyword - | sym::masked | sym::no_default_passes - | sym::no_inline | sym::notable_trait | sym::passes - | sym::plugins - | sym::fake_variadic => {} + | sym::plugins => {} sym::rust_logo => { - if !self.tcx.features().rustdoc_internals { + if self.check_attr_crate_level(attr, meta, hir_id) + && !self.tcx.features().rustdoc_internals + { feature_err( &self.tcx.sess, sym::rustdoc_internals, @@ -1235,12 +1137,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } - sym::test => { - if !self.check_test_attr(meta, hir_id) { - is_valid = false; - } - } - _ => { let path = rustc_ast_pretty::pprust::path_to_string(&i_meta.path); if i_meta.has_name(sym::spotlight) { @@ -1282,7 +1178,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { errors::DocTestUnknownAny { path }, ); } - is_valid = false; } } } else { @@ -1292,79 +1187,60 @@ impl<'tcx> CheckAttrVisitor<'tcx> { meta.span(), errors::DocInvalid, ); - is_valid = false; } } } - - is_valid } /// Warns against some misuses of `#[pass_by_value]` - fn check_pass_by_value(&self, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_pass_by_value(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Struct | Target::Enum | Target::TyAlias => true, + Target::Struct | Target::Enum | Target::TyAlias => {} _ => { self.dcx().emit_err(errors::PassByValue { attr_span: attr.span, span }); - false } } } - fn check_allow_incoherent_impl(&self, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_allow_incoherent_impl(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Method(MethodKind::Inherent) => true, + Target::Method(MethodKind::Inherent) => {} _ => { self.dcx().emit_err(errors::AllowIncoherentImpl { attr_span: attr.span, span }); - false } } } - fn check_has_incoherent_inherent_impls( - &self, - attr: &Attribute, - span: Span, - target: Target, - ) -> bool { + fn check_has_incoherent_inherent_impls(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Trait | Target::Struct | Target::Enum | Target::Union | Target::ForeignTy => { - true - } + Target::Trait | Target::Struct | Target::Enum | Target::Union | Target::ForeignTy => {} _ => { self.tcx .dcx() .emit_err(errors::HasIncoherentInherentImpl { attr_span: attr.span, span }); - false } } } - fn check_ffi_pure(&self, attr_span: Span, attrs: &[Attribute], target: Target) -> bool { + fn check_ffi_pure(&self, attr_span: Span, attrs: &[Attribute], target: Target) { if target != Target::ForeignFn { self.dcx().emit_err(errors::FfiPureInvalidTarget { attr_span }); - return false; + return; } if attrs.iter().any(|a| a.has_name(sym::ffi_const)) { // `#[ffi_const]` functions cannot be `#[ffi_pure]` self.dcx().emit_err(errors::BothFfiConstAndPure { attr_span }); - false - } else { - true } } - fn check_ffi_const(&self, attr_span: Span, target: Target) -> bool { - if target == Target::ForeignFn { - true - } else { + fn check_ffi_const(&self, attr_span: Span, target: Target) { + if target != Target::ForeignFn { self.dcx().emit_err(errors::FfiConstInvalidTarget { attr_span }); - false } } /// Warns against some misuses of `#[must_use]` - fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) -> bool { + fn check_must_use(&self, hir_id: HirId, attr: &Attribute, target: Target) { if !matches!( target, Target::Fn @@ -1397,23 +1273,19 @@ impl<'tcx> CheckAttrVisitor<'tcx> { errors::MustUseNoEffect { article, target }, ); } - - // For now, its always valid - true } - /// Checks if `#[must_not_suspend]` is applied to a function. Returns `true` if valid. - fn check_must_not_suspend(&self, attr: &Attribute, span: Span, target: Target) -> bool { + /// Checks if `#[must_not_suspend]` is applied to a function. + fn check_must_not_suspend(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Struct | Target::Enum | Target::Union | Target::Trait => true, + Target::Struct | Target::Enum | Target::Union | Target::Trait => {} _ => { self.dcx().emit_err(errors::MustNotSuspend { attr_span: attr.span, span }); - false } } } - /// Checks if `#[cold]` is applied to a non-function. Returns `true` if valid. + /// Checks if `#[cold]` is applied to a non-function. fn check_cold(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { match target { Target::Fn | Target::Method(..) | Target::ForeignFn | Target::Closure => {} @@ -1489,21 +1361,19 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } - /// Checks if `#[no_link]` is applied to an `extern crate`. Returns `true` if valid. - fn check_no_link(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) -> bool { + /// Checks if `#[no_link]` is applied to an `extern crate`. + fn check_no_link(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { match target { - Target::ExternCrate => true, + Target::ExternCrate => {} // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[no_link]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { self.inline_attr_str_error_with_macro_def(hir_id, attr, "no_link"); - true } _ => { self.dcx().emit_err(errors::NoLink { attr_span: attr.span, span }); - false } } } @@ -1512,57 +1382,42 @@ impl<'tcx> CheckAttrVisitor<'tcx> { matches!(self.tcx.hir_node(hir_id), hir::Node::ImplItem(..)) } - /// Checks if `#[export_name]` is applied to a function or static. Returns `true` if valid. - fn check_export_name( - &self, - hir_id: HirId, - attr: &Attribute, - span: Span, - target: Target, - ) -> bool { + /// Checks if `#[export_name]` is applied to a function or static. + fn check_export_name(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) { match target { - Target::Static | Target::Fn => true, - Target::Method(..) if self.is_impl_item(hir_id) => true, + Target::Static | Target::Fn => {} + Target::Method(..) if self.is_impl_item(hir_id) => {} // FIXME(#80564): We permit struct fields, match arms and macro defs to have an // `#[export_name]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { self.inline_attr_str_error_with_macro_def(hir_id, attr, "export_name"); - true } _ => { self.dcx().emit_err(errors::ExportName { attr_span: attr.span, span }); - false } } } - fn check_rustc_layout_scalar_valid_range( - &self, - attr: &Attribute, - span: Span, - target: Target, - ) -> bool { + fn check_rustc_layout_scalar_valid_range(&self, attr: &Attribute, span: Span, target: Target) { if target != Target::Struct { self.dcx().emit_err(errors::RustcLayoutScalarValidRangeNotStruct { attr_span: attr.span, span, }); - return false; + return; } let Some(list) = attr.meta_item_list() else { - return false; + return; }; - if matches!(&list[..], &[NestedMetaItem::Lit(MetaItemLit { kind: LitKind::Int(..), .. })]) { - true - } else { + if !matches!(&list[..], &[NestedMetaItem::Lit(MetaItemLit { kind: LitKind::Int(..), .. })]) + { self.tcx .dcx() .emit_err(errors::RustcLayoutScalarValidRangeArg { attr_span: attr.span }); - false } } @@ -1574,7 +1429,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { span: Span, target: Target, item: Option>, - ) -> bool { + ) { let is_function = matches!(target, Target::Fn); if !is_function { self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { @@ -1582,12 +1437,12 @@ impl<'tcx> CheckAttrVisitor<'tcx> { defn_span: span, on_crate: hir_id == CRATE_HIR_ID, }); - return false; + return; } let Some(list) = attr.meta_item_list() else { // The attribute form is validated on AST. - return false; + return; }; let Some(ItemLike::Item(Item { @@ -1605,7 +1460,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr_span: attr.span, param_span: param.span, }); - return false; + return; } } } @@ -1615,7 +1470,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr_span: attr.span, generics_span: generics.span, }); - return false; + return; } let arg_count = decl.inputs.len() as u128 + generics.params.len() as u128; @@ -1628,7 +1483,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { span, arg_count: arg_count as usize, }); - return false; + return; } } else { invalid_args.push(meta.span()); @@ -1637,9 +1492,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { if !invalid_args.is_empty() { self.dcx().emit_err(errors::RustcLegacyConstGenericsIndexNegative { invalid_args }); - false - } else { - true } } @@ -1651,7 +1503,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: &Attribute, span: Span, target: Target, - ) -> bool { + ) { let is_function = matches!(target, Target::Fn | Target::Method(..)); if !is_function { self.dcx().emit_err(errors::AttrShouldBeAppliedToFn { @@ -1659,9 +1511,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> { defn_span: span, on_crate: hir_id == CRATE_HIR_ID, }); - false - } else { - true } } @@ -1673,7 +1522,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: &Attribute, span: Span, target: Target, - ) -> bool { + ) { self.check_applied_to_fn_or_method(hir_id, attr, span, target) } @@ -1685,60 +1534,49 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: &Attribute, span: Span, target: Target, - ) -> bool { + ) { self.check_applied_to_fn_or_method(hir_id, attr, span, target) } /// Checks that the `#[rustc_lint_opt_ty]` attribute is only applied to a struct. - fn check_rustc_lint_opt_ty(&self, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_rustc_lint_opt_ty(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Struct => true, + Target::Struct => {} _ => { self.dcx().emit_err(errors::RustcLintOptTy { attr_span: attr.span, span }); - false } } } /// Checks that the `#[rustc_lint_opt_deny_field_access]` attribute is only applied to a field. - fn check_rustc_lint_opt_deny_field_access( - &self, - attr: &Attribute, - span: Span, - target: Target, - ) -> bool { + fn check_rustc_lint_opt_deny_field_access(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Field => true, + Target::Field => {} _ => { self.tcx .dcx() .emit_err(errors::RustcLintOptDenyFieldAccess { attr_span: attr.span, span }); - false } } } /// Checks that the dep-graph debugging attributes are only present when the query-dep-graph /// option is passed to the compiler. - fn check_rustc_dirty_clean(&self, attr: &Attribute) -> bool { - if self.tcx.sess.opts.unstable_opts.query_dep_graph { - true - } else { + fn check_rustc_dirty_clean(&self, attr: &Attribute) { + if !self.tcx.sess.opts.unstable_opts.query_dep_graph { self.dcx().emit_err(errors::RustcDirtyClean { span: attr.span }); - false } } /// Checks if the attribute is applied to a trait. - fn check_must_be_applied_to_trait(&self, attr: &Attribute, span: Span, target: Target) -> bool { + fn check_must_be_applied_to_trait(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Trait => true, + Target::Trait => {} _ => { self.dcx().emit_err(errors::AttrShouldBeAppliedToTrait { attr_span: attr.span, defn_span: span, }); - false } } } @@ -2036,43 +1874,38 @@ impl<'tcx> CheckAttrVisitor<'tcx> { span: Span, target: Target, attrs: &[Attribute], - ) -> bool { + ) { debug!("Checking target: {:?}", target); match target { Target::Fn => { for attr in attrs { if attr.is_proc_macro_attr() { debug!("Is proc macro attr"); - return true; + return; } } debug!("Is not proc macro attr"); - false } - Target::MacroDef => true, + Target::MacroDef => {} // FIXME(#80564): We permit struct fields and match arms to have an // `#[allow_internal_unstable]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. - Target::Field | Target::Arm => { - self.inline_attr_str_error_without_macro_def( - hir_id, - attr, - "allow_internal_unstable", - ); - true - } + Target::Field | Target::Arm => self.inline_attr_str_error_without_macro_def( + hir_id, + attr, + "allow_internal_unstable", + ), _ => { self.tcx .dcx() .emit_err(errors::AllowInternalUnstable { attr_span: attr.span, span }); - false } } } /// Checks if the items on the `#[debugger_visualizer]` attribute are valid. - fn check_debugger_visualizer(&self, attr: &Attribute, target: Target) -> bool { + fn check_debugger_visualizer(&self, attr: &Attribute, target: Target) { // Here we only check that the #[debugger_visualizer] attribute is attached // to nothing other than a module. All other checks are done in the // `debugger_visualizer` query where they need to be done for decoding @@ -2081,11 +1914,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> { Target::Mod => {} _ => { self.dcx().emit_err(errors::DebugVisualizerPlacement { span: attr.span }); - return false; } } - - true } /// Outputs an error for `#[allow_internal_unstable]` which can only be applied to macros. @@ -2096,26 +1926,21 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: &Attribute, span: Span, target: Target, - ) -> bool { + ) { match target { Target::Fn | Target::Method(_) - if self.tcx.is_const_fn_raw(hir_id.expect_owner().to_def_id()) => - { - true - } + if self.tcx.is_const_fn_raw(hir_id.expect_owner().to_def_id()) => {} // FIXME(#80564): We permit struct fields and match arms to have an // `#[allow_internal_unstable]` attribute with just a lint, because we previously // erroneously allowed it and some crates used it accidentally, to be compatible // with crates depending on them, we can't throw an error here. Target::Field | Target::Arm | Target::MacroDef => { - self.inline_attr_str_error_with_macro_def(hir_id, attr, "allow_internal_unstable"); - true + self.inline_attr_str_error_with_macro_def(hir_id, attr, "allow_internal_unstable") } _ => { self.tcx .dcx() .emit_err(errors::RustcAllowConstFnUnstable { attr_span: attr.span, span }); - false } } } @@ -2126,65 +1951,56 @@ impl<'tcx> CheckAttrVisitor<'tcx> { attr: &Attribute, span: Span, target: Target, - ) -> bool { + ) { if let Target::ForeignFn = target && let hir::Node::Item(Item { kind: ItemKind::ForeignMod { abi: Abi::RustIntrinsic, .. }, .. }) = self.tcx.parent_hir_node(hir_id) { - return true; + return; } self.dcx().emit_err(errors::RustcSafeIntrinsic { attr_span: attr.span, span }); - false } - fn check_rustc_std_internal_symbol( - &self, - attr: &Attribute, - span: Span, - target: Target, - ) -> bool { + fn check_rustc_std_internal_symbol(&self, attr: &Attribute, span: Span, target: Target) { match target { - Target::Fn | Target::Static => true, + Target::Fn | Target::Static => {} _ => { self.tcx .dcx() .emit_err(errors::RustcStdInternalSymbol { attr_span: attr.span, span }); - false } } } - fn check_stability_promotable(&self, attr: &Attribute, target: Target) -> bool { + fn check_stability_promotable(&self, attr: &Attribute, target: Target) { match target { Target::Expression => { self.dcx().emit_err(errors::StabilityPromotable { attr_span: attr.span }); - false } - _ => true, + _ => {} } } - fn check_link_ordinal(&self, attr: &Attribute, _span: Span, target: Target) -> bool { + fn check_link_ordinal(&self, attr: &Attribute, _span: Span, target: Target) { match target { - Target::ForeignFn | Target::ForeignStatic => true, + Target::ForeignFn | Target::ForeignStatic => {} _ => { self.dcx().emit_err(errors::LinkOrdinal { attr_span: attr.span }); - false } } } - fn check_confusables(&self, attr: &Attribute, target: Target) -> bool { + fn check_confusables(&self, attr: &Attribute, target: Target) { match target { Target::Method(MethodKind::Inherent) => { let Some(meta) = attr.meta() else { - return false; + return; }; let ast::MetaItem { kind: MetaItemKind::List(ref metas), .. } = meta else { - return false; + return; }; let mut candidates = Vec::new(); @@ -2198,21 +2014,17 @@ impl<'tcx> CheckAttrVisitor<'tcx> { hi: meta.span().shrink_to_hi(), }, }); - return false; + return; }; candidates.push(meta_lit.symbol); } if candidates.is_empty() { self.dcx().emit_err(errors::EmptyConfusables { span: attr.span }); - return false; } - - true } _ => { self.dcx().emit_err(errors::Confusables { attr_span: attr.span }); - false } } }