From 829c95673094e93e24c038c1745aa718353f7173 Mon Sep 17 00:00:00 2001 From: John Bobbo Date: Sat, 15 Apr 2023 20:42:33 -0700 Subject: [PATCH] Add the `inlined_generics` lint, which warns about functions/methods with generics which are also marked either `#[inline]` or `#[inline(always)]`. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/inlined_generics.rs | 174 +++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 3 + tests/ui/inlined_generics.rs | 42 +++++++ tests/ui/inlined_generics.stderr | 40 ++++++ 6 files changed, 261 insertions(+) create mode 100644 clippy_lints/src/inlined_generics.rs create mode 100644 tests/ui/inlined_generics.rs create mode 100644 tests/ui/inlined_generics.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 559b560dde4b..067f08b35d61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4600,6 +4600,7 @@ Released 2018-09-13 [`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax [`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax [`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body +[`inlined_generics`]: https://rust-lang.github.io/rust-clippy/master/index.html#inlined_generics [`inspect_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#inspect_for_each [`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one [`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index f24dab627809..9ee85d2f6e29 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -209,6 +209,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY_INFO, crate::init_numbered_fields::INIT_NUMBERED_FIELDS_INFO, crate::inline_fn_without_body::INLINE_FN_WITHOUT_BODY_INFO, + crate::inlined_generics::INLINED_GENERICS_INFO, crate::instant_subtraction::MANUAL_INSTANT_ELAPSED_INFO, crate::instant_subtraction::UNCHECKED_DURATION_SUBTRACTION_INFO, crate::int_plus_one::INT_PLUS_ONE_INFO, diff --git a/clippy_lints/src/inlined_generics.rs b/clippy_lints/src/inlined_generics.rs new file mode 100644 index 000000000000..1f23502c84bd --- /dev/null +++ b/clippy_lints/src/inlined_generics.rs @@ -0,0 +1,174 @@ +use rustc_attr::InlineAttr; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{AssocItemKind, Body, FnDecl, GenericParam, GenericParamKind, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; + +use clippy_utils::diagnostics::span_lint; + +declare_clippy_lint! { + /// ### What it does + /// It lints any generic functions, or methods which are marked with + /// the `#[inline]`, or `#[inline(always)]` attributes. + /// + /// ### Why is this bad? + /// It's not inherently bad to mark generic functions, or methods with + /// the `#[inline]`, or `#[inline(always)]` attributes, but it can possibly + /// increase compilation times, because the compiler will already monomorphize + /// generic functions per-crate, while inlining a function/method will also + /// cause the compiler to recompile per code-gen unit, which may cause even + /// longer compile times. + /// + /// ### Example + /// ```rust + /// #[inline] + /// fn foo(_: F) {} // generic function is marked `#[inline]` + /// + /// #[inline(always)] + /// fn bar(_: B) {} // generic function is marked `#[inline(always)]` + /// + /// struct Foo { + /// str: String, + /// } + /// + /// impl Foo { + /// #[inline] // generic method is marked `#[inline]` + /// fn new>(str: S) -> Self { + /// Self { + /// str: str.as_ref().to_owned(), + /// } + /// } + /// } + /// ``` + /// Use instead: + /// ```rust + /// fn foo(_: F) {} + /// + /// fn bar(_: B) {} + /// + /// struct Foo { + /// str: String, + /// } + /// + /// impl Foo { + /// fn new>(str: S) -> Self { + /// Self { + /// str: str.as_ref().to_owned(), + /// } + /// } + /// } + /// ``` + #[clippy::version = "1.70.0"] + pub INLINED_GENERICS, + restriction, + "detects generic functions, or methods which are marked `#[inline]`, or `#[inline(always)]`" +} + +declare_lint_pass!(InlinedGenerics => [INLINED_GENERICS]); + +fn requires_monomorphization<'hir>(params: &'hir [GenericParam<'hir>]) -> bool { + params.iter().any(|param| { + matches!( + param.kind, + GenericParamKind::Type { .. } | GenericParamKind::Const { .. } + ) + }) +} + +fn lint_inlined_generics(ctx: &LateContext<'_>, span: Span, desc: &'static str, inline: &'static str) { + span_lint( + ctx, + INLINED_GENERICS, + span, + &format!("generic {desc} is marked `{inline}`"), + ); +} + +impl<'tcx> LateLintPass<'tcx> for InlinedGenerics { + fn check_fn( + &mut self, + ctx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _: &'tcx FnDecl<'tcx>, + _: &'tcx Body<'tcx>, + span: Span, + def_id: LocalDefId, + ) { + if !ctx.tcx.generics_of(def_id).own_requires_monomorphization() { + return; + } + + let inline = match ctx.tcx.codegen_fn_attrs(def_id).inline { + InlineAttr::Never | InlineAttr::None => return, + InlineAttr::Always => "#[inline(always)]", + InlineAttr::Hint => "#[inline]", + }; + match kind { + FnKind::ItemFn(..) => { + lint_inlined_generics(ctx, span, "function", inline); + }, + FnKind::Method(..) => { + lint_inlined_generics(ctx, span, "method", inline); + }, + FnKind::Closure => {}, + } + } + + fn check_item(&mut self, ctx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + match item.kind { + ItemKind::Trait(.., generics, _, items) => { + let monomorphize = requires_monomorphization(generics.params); + for item in items { + let def_id = item.id.owner_id.def_id; + if !(monomorphize || ctx.tcx.generics_of(def_id).own_requires_monomorphization()) { + continue; + } + + if let AssocItemKind::Fn { has_self: true } = item.kind { + let inline = match ctx.tcx.codegen_fn_attrs(def_id).inline { + InlineAttr::Never | InlineAttr::None => continue, + InlineAttr::Always => "#[inline(always)]", + InlineAttr::Hint => "#[inline]", + }; + lint_inlined_generics(ctx, item.span, "trait method", inline); + } + } + }, + ItemKind::Impl(impl_block) => { + let monomorphize = requires_monomorphization(impl_block.generics.params); + for item in impl_block.items { + let def_id = item.id.owner_id.def_id; + if !(monomorphize || ctx.tcx.generics_of(def_id).own_requires_monomorphization()) { + continue; + } + + if let AssocItemKind::Fn { has_self: true } = item.kind { + let inline = match ctx.tcx.codegen_fn_attrs(def_id).inline { + InlineAttr::Never | InlineAttr::None => continue, + InlineAttr::Always => "#[inline(always)]", + InlineAttr::Hint => "#[inline]", + }; + lint_inlined_generics(ctx, item.span, "method", inline); + } + } + }, + ItemKind::ExternCrate(..) + | ItemKind::Use(..) + | ItemKind::Static(..) + | ItemKind::Const(..) + | ItemKind::Fn(..) + | ItemKind::Macro(..) + | ItemKind::Mod(..) + | ItemKind::ForeignMod { .. } + | ItemKind::GlobalAsm(..) + | ItemKind::TyAlias(..) + | ItemKind::OpaqueTy(..) + | ItemKind::Enum(..) + | ItemKind::Struct(..) + | ItemKind::Union(..) + | ItemKind::TraitAlias(..) => {}, + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b0ec14855e71..0e0a068cb33a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -24,6 +24,7 @@ extern crate rustc_arena; extern crate rustc_ast; extern crate rustc_ast_pretty; +extern crate rustc_attr; extern crate rustc_data_structures; extern crate rustc_driver; extern crate rustc_errors; @@ -153,6 +154,7 @@ mod inherent_impl; mod inherent_to_string; mod init_numbered_fields; mod inline_fn_without_body; +mod inlined_generics; mod instant_subtraction; mod int_plus_one; mod invalid_upcast_comparisons; @@ -959,6 +961,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule)); store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation)); store.register_early_pass(|| Box::new(suspicious_doc_comments::SuspiciousDocComments)); + store.register_late_pass(|_| Box::new(inlined_generics::InlinedGenerics)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/inlined_generics.rs b/tests/ui/inlined_generics.rs new file mode 100644 index 000000000000..90f31715bb96 --- /dev/null +++ b/tests/ui/inlined_generics.rs @@ -0,0 +1,42 @@ +#![allow(unused)] +#![warn(clippy::inlined_generics)] + +trait Foo { + fn foo(&self, t: T); + + #[inline] + fn bar(&self, t: T) { + self.foo(t); + } +} + +impl Foo for T { + #[inline(always)] + fn foo(&self, t: T) {} + + #[inline(never)] // This is ignored. + fn bar(&self, t: T) {} +} + +struct FooBar; + +impl FooBar { + #[inline] + fn baz(t: T) {} + + #[inline(never)] + fn qux(t: T) {} +} + +#[inline] +fn foo(t: T) {} +#[inline(always)] +fn bar(t: T) +where + T: Clone, +{ +} +#[inline(never)] // Also ignored. +fn baz(t: T) {} + +fn main() {} diff --git a/tests/ui/inlined_generics.stderr b/tests/ui/inlined_generics.stderr new file mode 100644 index 000000000000..1f0f0b9c1997 --- /dev/null +++ b/tests/ui/inlined_generics.stderr @@ -0,0 +1,40 @@ +error: generic trait method is marked `#[inline]` + --> $DIR/inlined_generics.rs:8:5 + | +LL | / fn bar(&self, t: T) { +LL | | self.foo(t); +LL | | } + | |_____^ + | + = note: `-D clippy::inlined-generics` implied by `-D warnings` + +error: generic method is marked `#[inline(always)]` + --> $DIR/inlined_generics.rs:15:5 + | +LL | fn foo(&self, t: T) {} + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: generic method is marked `#[inline]` + --> $DIR/inlined_generics.rs:25:5 + | +LL | fn baz(t: T) {} + | ^^^^^^^^^^^^^^^^^^ + +error: generic function is marked `#[inline]` + --> $DIR/inlined_generics.rs:32:1 + | +LL | fn foo(t: T) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: generic function is marked `#[inline(always)]` + --> $DIR/inlined_generics.rs:34:1 + | +LL | / fn bar(t: T) +LL | | where +LL | | T: Clone, +LL | | { +LL | | } + | |_^ + +error: aborting due to 5 previous errors +