Warn about missing `#[inline]` on trivial public methods

[emilk]

Warn about missing `#[inline]` on trivial public methods

[GuillaumeGomez]

It could be potentially expanded to handled cases like an if/else condition with one expression inside each branch. For now, I'm going to implement the lint for this simple case.

[y21]

A lot of people don't realize that #[inline] is also important when implementing simple trait functions like these, so that's why I'm calling it out explicitly.

Is this really still true today? As of recently rustc has started automatically adding #[inline] to all "trivial" functions so it also works for non-LTO builds (this also broke compiler-explorer, which now requires #[inline(never)] for functions to actually show up). IMHO any "trivial" function that rustc doesn't inline (and where it would be worth it) seems like a bug in its cost checker and it'd make more sense to me to just fix it in the MIR inliner instead of telling users to pollute their code with #[inline] annotations all over the place

[emilk]

I agree with you in priciple y21, but last I read about the auto-inlining in rustc it was extremely simple. Maybe it has improved though (and maybe I should open an issue on rustc instead).

Does anyone know the unit-tests for this is for rustc, so we can check which cases are actually handled?

[y21]

The PR that improved it is rust-lang/rust#116505. It adds the -Zcross-crate-inline-threshold flag, by default it currently has the value 100.
That value is the max. number of MIR statements to consider "trivial" and to allow when considering if a function should be marked as inlineable. 100 statements should be enough for functions that simply return a field, construct a value, return an argument or do some small transformation to it, so it should indeed at least catch 3/4 cases mentioned.

The exact criteria are:
https://github.com/rust-lang/rust/blob/4a78c00e227124ff9d5ece2d493472e7325c87d3/compiler/rustc_mir_transform/src/cross_crate_inline.rs#L82-L85

Interestingly, that checker.calls == 0 condition does look like it would prevent the "does at most one function call" case from being inlined. I'm not sure if there's some technical reason for it I'm not aware of or if it could be tuned to return true for calls == 1 && statements == 0 specifically.

But anyway, if there are other trivial functions that aren't caught by this I do think an issue on the rust issue tracker would be better. Everyone would benefit from better heuristics in that area.

A clippy lint would duplicate what's already done in rustc but just with different heuristics that may or may not be better. There are some subtleties too that would need to be taken into account, which Rust's inliner does, such as being careful if a field access invokes Deref code, or if any parameters are dropped and expensive Drop code is executed at the end of the scope, ...
If the MIR cost checker something shouldn't be inlined, I don't think that clippy should say it either.