Extend declare_lint_pass! macro

[BD103]

Often lints need to compare Symbols. Some lints take the inefficient route by interning the string each time:

if ident.name == Symbol::intern("component") {
    // ...
}

// Also seen as...
if ident.name == sym!(component) {
    // ...
}

This is bad because interning a string is quite expensive. To avoid this, lint passes cache the Symbol:

struct MyLintPass {
    component_sym: Symbol,
}

impl Default for MyLintPass {
    fn default() -> Self {
        Self { component_sym: Symbol::intern("component") }
    }
}

impl<'tcx> LateLintPass<'tcx> for MyLintPass {
    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
        // ...

        if ident.name == self.component_sym {
            // ...
        }
    }
}

This requires a lot of boilerplate. To shorten this, this PR introduces a custom declare_bevy_lint_pass! macro that supports cached values:

declare_bevy_lint_pass! {
    MyLintPass => [MY_LINT.lint],
    @default = {
        component: Symbol = sym!("component"),
    },
}

Eventually this macro will be used to declare custom lint config, but for now it just supports default values. I've migrated all existing lints to use declare_bevy_lint_pass!.

[TimJentzsch]

In general looks good to me, although I'm still bad with the macro syntax, so can't say much about that (seriously, probably one of the worst looking parts of Rust, IMO).

I'm curious: Do you have any benchmarks (even if it's just a simple timing of running the linter on some random repo) by how much this improves the performance?

[DaAlbrecht]

Looks really good, makes defining lints so much easier!!

[BD103]

I'm curious: Do you have any benchmarks (even if it's just a simple timing of running the linter on some random repo) by how much this improves the performance?

For the Symbol::intern() optimization, it avoids¹:

1. Locking a thread-local
2. Looking up the symbol string in a hash set
3. Allocating space for the symbol (which may re-allocate the arena if it's out of room)

Instead, it's a simple u32 == u32 comparison, which should be much faster. I don't have exact benchmarks, since I ran into some issues running criterion in #![feature(rustc_private)] mode, but I may be able to get them working if you want them.

I don't think caching symbols will have visible performance effects on the linter right now, but they may add up if we have as many lints as Clippy does.

---

On another note, we don't have benchmarks tracking the time of the entire linter being run on a project. I would like to setup something like that eventually, so we can track the performance of individual lints.

Footnotes

1. See Symbol::intern(), Interner, and Interner::intern() ↩

[TimJentzsch]

Thanks, then it's fine for me now.

Let's create a follow-up issue to set up some basic way to compare performance, even if it's only logging the time instead of comprehensive benchmarking like with criterion