Audit and handle remaining cases of `rustc::potential_query_instability` lint (iterating `HashMap`s)

[Aaron1011]

The iteration order of a HashMap depends on the Hash value of the keys. In rustc, the Hash value of a key can change between compilation sessions, even if the HashStable value remains the same (e.g. DefId, Symbol, etc.). This means that iterating over a HashMap can cause a query to produce different results given the same (as determined by HashStable) input. This can be quite subtle, as demonstrated by #82272 (comment)

To eliminate this source of issues, we should make an internal rustc-only lint for iterating over a HashMap in any way. This will need to cover explicit method calls (e.g. .keys() and .values()), as well as any usage of the IntoIterator impl for HashMap.

Note that we don't want to ban the usage of HashMaps with unstable keys - as long as they're only used for lookup (and not iteration), it's much more efficient than repeatedly converting to the stable form of a key (e.g. DefId -> DefPathHash or Symbol -> String).

[bjorn3]

Note that we don't want to ban the usage of HashMaps with unstable keys - as long as they're only used for lookup (and not iteration),

#64131 added the StableMap wrapper for FxHashMap that doesn't expose the iteration order.

[jyn514]

@bjorn3 see #84446

[cjgillot]

The lint exists since #89558, and has allowed on all compiler crates as rustc::potential_query_instability.
We now need to stop allowing it in the compiler.

Steps, for each crate except rustc_data_structures:

• remove the allow(rustc::potential_query_instability) in lib.rs;
• replace the FxHash{Map,Set} by FxIndex{Map,Set} where necessary.

Please reach out on zulip if there are any questions.

[fee1-dead]

@rustbot claim

@rustbot release-assignment

[NiklasJonsson]

It seems like no one is assigned to this and I'd like to give it a try so assigning myself.

@rustbot claim