Clippy stores Symbols in lazy_static

[oli-obk]

Citing @ehuss in #60819 (comment)

Clippy is storing Symbol definitions in globals (lazy_static), but those symbol definitions are only valid for a single session/thread. When rls fires up a second build, clippy reuses those Symbol definitions, but they point to invalid values.

I'm not sure of the best way to fix that. I'm pretty sure Clippy shouldn't be holding any global references to Symbols. Maybe clippy_lints/src/utils/paths.rs could go back to using strs and create Symbols as needed? I'm not sure how much of a performance issue that would be, or if there's a better way.

I assumed that rls kills only the 'tcx interners and restarts at TyCtxt creation time, but that assumption is apparently wrong.

I don't want clippy to diverge from rustc's Symbol strategy and there's more than just paths that we're using them for (especially since more APIs are being turned into symbol-using ones).

One idea would be for rustc's symbols! macro invocation to produce a macro (clippy_symbols), which not only allows us to create a list of symbols statically by simply reserving 1000 symbols or so, but also has a list of all the already existing symbols and makes sure we don't duplicate any. The deduplication is currently achieved by Symbol::intern being called in lazy statics.

Alternatively we could probably setup some way for rls to tell its rustc invocation to only create a symbol table if there is already one and to not nuke the existing one on shutdown. This seems like the best short term option to me if there are no technical problems with it.

[oli-obk]

cc @Manishearth

[petrochenkov]

The symbol list in rustc is pre-filled during Globals::new in with_globals.
Perhaps Interner::prefill could also intern an extra list of symbols provided through some callback in rustc_interface.

[oli-obk]

how would I expose this list of symbols to my lints so that I can access individual symbols by name?

[petrochenkov]

// Defined by clippy (probably generated by a macro)
mod clippy_sim {
    pub const my_sim1: Symbol = Symbol::from_u32(Symbol::NUM_PREINTERNED + 0);
    pub const my_sim2: Symbol = Symbol::from_u32(Symbol::NUM_PREINTERNED + 1);
    pub const my_sim3: Symbol = Symbol::from_u32(Symbol::NUM_PREINTERNED + 2);
    ...
}

fn rustc_interface_prefill_extra_callback() -> &[&str] { // Defined by clippy, called by `Interner::prefill` in rustc
    &["my_sim1", "my_sim2", "my_sim3", ...]
}

[Manishearth]

Should I take a crack at #39131 first? That's the real fix to most of our usage of Symbol, and if we're hardcoding this in rustc we might as well do it right.

I suspect we'll have a couple left, though.

[oli-obk]

I suspect we'll have a couple left, though.

That and the worry that implementing #39131 will be quite a bit of work and thus block rls for a week or more makes me prefer to do @petrochenkov's solution now, and address diagnostic items in its own timeframe.

[Manishearth]

Oh, yeah, go ahead with any RLS quickfixes first. But we should try that out before trying to make this fix good.