Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the inlined_generics lint. #10650

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
174 changes: 174 additions & 0 deletions clippy_lints/src/inlined_generics.rs
Original file line number Diff line number Diff line change
@@ -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>(_: F) {} // generic function is marked `#[inline]`
///
/// #[inline(always)]
/// fn bar<B>(_: B) {} // generic function is marked `#[inline(always)]`
///
/// struct Foo {
/// str: String,
/// }
///
/// impl Foo {
/// #[inline] // generic method is marked `#[inline]`
/// fn new<S: AsRef<str>>(str: S) -> Self {
/// Self {
/// str: str.as_ref().to_owned(),
/// }
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// fn foo<F>(_: F) {}
///
/// fn bar<B>(_: B) {}
///
/// struct Foo {
/// str: String,
/// }
///
/// impl Foo {
/// fn new<S: AsRef<str>>(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(..) => {},
}
}
}
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
42 changes: 42 additions & 0 deletions tests/ui/inlined_generics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#![allow(unused)]
#![warn(clippy::inlined_generics)]

trait Foo<T> {
fn foo(&self, t: T);

#[inline]
fn bar(&self, t: T) {
self.foo(t);
}
}

impl<T> Foo<T> 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: T) {}

#[inline(never)]
fn qux<T>(t: T) {}
}

#[inline]
fn foo<T: Copy>(t: T) {}
#[inline(always)]
fn bar<T>(t: T)
where
T: Clone,
{
}
#[inline(never)] // Also ignored.
fn baz<T>(t: T) {}

fn main() {}
40 changes: 40 additions & 0 deletions tests/ui/inlined_generics.stderr
Original file line number Diff line number Diff line change
@@ -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: T) {}
| ^^^^^^^^^^^^^^^^^^

error: generic function is marked `#[inline]`
--> $DIR/inlined_generics.rs:32:1
|
LL | fn foo<T: Copy>(t: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: generic function is marked `#[inline(always)]`
--> $DIR/inlined_generics.rs:34:1
|
LL | / fn bar<T>(t: T)
LL | | where
LL | | T: Clone,
LL | | {
LL | | }
| |_^

error: aborting due to 5 previous errors