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

Opt-out of lint #[inline] on generic functions #11187

Open
appetrosyan opened this issue Jul 19, 2023 · 1 comment
Open

Opt-out of lint #[inline] on generic functions #11187

appetrosyan opened this issue Jul 19, 2023 · 1 comment

Comments

@appetrosyan
Copy link

Description

The lint is far from ergonomic because of the rate of false-positives.

Applying this lint is equivalent to applying LTO to just one crate, badly.

I propose making it a little easier to use by opting out of cases where it might be unapplicable as a class of functions.

One shape this could take is splitting the lint up into several.

  • missing_inline_for_public_trivial_fn. This is the lint that most people want, because it looks at the cyclomatic complexity of a function and suggests that it be inline.
  • missing_inline_for_constructor, which matches a pattern where a function (not necessarily named new, accepts one parameter, and constructs another struct. Regardless of how long that actually is in code, if the AST is exported to another crate LLVM can do a good job of optimising it. The inlining hint is also useful.
  • missing_inline_for_generic_fn, which is the lint that I would not use personally. There are cases where the generic monomorphisation also needs a hint to the compiler that it needs to be inline(always) lint #[inline] on generic functions #2253. Experience from C++ suggests that LLVM can do a better job of knowing which monomorphisations to inline and which to keep as separate functions.

This is a great oversimplification of the more-art-than-science of determining which functions need to be annotated as inline, but it highlights two cases where I would definitely say "it should be #[inline]" and cases where I'd say "it probably shouldn't".

Having these as separate lints would allow me to do something like deny(missing_inline_for_public_trivial_fn), and warn(missing_inline_for_constructor, missing_inline_for_generic_fn), or apply them more granularly across different modules (e.g. a module where all generics are also inline(always) and a module where items are not.

Version

No response

Additional Labels

No response

@emilk
Copy link

emilk commented May 14, 2024

I also want a missing_inline_for_public_trivial_fn lint. Currently I have a custom Python script that reminds me to add #[inline] to these cases:

  • Trivial builder methods (pub fn with_foo(mut self, foo: Foo) -> Self { self.foo = foo; self }
  • trait implementation that commonly just forward the call: deref, deref_mut, as_ref, borrow, …

For instance:

impl AsRef<str> for Utf8 {
    #[inline]
    fn as_ref(&self) -> &str {
        self.as_str()
    }
}

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants