Remove the unused StableSet and StableMap types from rustc_data_structures.

[michaelwoerister]

The current implementation is not "stable" in the same sense that HashStable and StableHasher are stable, i.e. across compilation sessions. So, in my opinion, it's better to remove those types (which are basically unused anyway) than to give the wrong impression that these are safe for incr. comp.

I plan to provide new "stable" collection types soon that can be used to replace FxHashMap and FxHashSet in query results (see draft). It's unsound that HashMap and HashSet implement HashStable (see #98890 for a recent P-critical bug caused by this) -- so we should make some progress there.

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

[rust-highfive]

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

[michaelwoerister]

See #94188 for some prior work in this direction. I'll probably create an MCP for removing the HashStable implementation from HashMap and HashSet.

[cjgillot]

#98890 was not caused by a FxHashMap getting returned by a query. It was caused by a FxHashMap getting iterated upon. I think the proper fix for this class of bugs is the internal lint forbidding iteration over hash-maps.

[michaelwoerister]

In the MCP write-up I'll explain in more detail how I think things fit together. But in short, I think it's preferable to just not implement HashStable for HashMap and HashSet (and BTreeMap and BTreeSet). They don't fulfill the contract that any observable difference results in a different Fingerprint -- so I think it's hard to argue that they should nonetheless implement HashStable.

I think the proper fix for this class of bugs is the internal lint forbidding iteration over hash-maps.

I think the solution is a combination of both. If we disallow types in query results that don't have a proper HashStable impl, then indeterminism coming from iterating over a HashSet will result in spurious cache invalidation instead of potential soundness errors -- so that's an improvement. But we don't want the spurious invalidations either, so the lint will be useful to avoid those too.

[nagisa]

@bors r+

[bors]

📌 Commit 2d2ac32 has been approved by nagisa

It is now in the queue for this repository.

[Dylan-DPC]

failed in rollup ci

[Dylan-DPC]

@bors r-

[bors]

☔ The latest upstream changes (presumably #99451) made this pull request unmergeable. Please resolve the merge conflicts.

[michaelwoerister]

Rebased.

Don't include this in rollups since it can easily break if another PR adds a use rustc_data_structures::stable_map::FxHashMap before this is being tested.

@bors r=nagisa rollup=never

[bors]

📌 Commit 88f6c6d has been approved by nagisa

It is now in the queue for this repository.

[bors]

⌛ Testing commit 88f6c6d with merge be9cfb3...

[bors]

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing be9cfb3 to master...

[rust-timer]

Finished benchmarking commit (be9cfb3): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

• Primary benchmarks: 🎉 relevant improvement found
• Secondary benchmarks: 😿 relevant regression found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.2%	2.2%	1
Improvements 🎉 (primary)	-2.7%	-2.7%	1
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-2.7%	-2.7%	1

Cycles

This benchmark run did not return any relevant results for this metric.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

1. the arithmetic mean of the percent change ↩

2. number of relevant changes ↩

[rust-log-analyzer]

The job x86_64-gnu-llvm-12 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling rustc_expand v0.0.0 (/checkout/compiler/rustc_expand)
error[E0282]: type annotations needed
   --> compiler/rustc_middle/src/mir/terminator.rs:294:42
    |
294 |             1 => write!(fmt, " -> {:?}", self.successors().next().unwrap()),
    |                  |                       |
    |                  |                       |
    |                  |                       cannot infer type of the type parameter `T` declared on the associated function `new_debug`
    |
   ::: /checkout/library/core/src/macros/mod.rs:498:1
    |
498 | macro_rules! write {
498 | macro_rules! write {
    | ------------------ in this expansion of `write!` (#1)
499 |     ($dst:expr, $($arg:tt)*) => {{
500 |         let result = $dst.write_fmt($crate::format_args!($($arg)*));
...
879 |     macro_rules! format_args {
    |     ------------------------ in this expansion of `$crate::format_args!` (#2)
    |
    |
help: consider specifying the generic argument
    |
294 |             1 => write!(fmt, " -> {:?}", self.successors().next().unwrap()::<T>),

   Compiling rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error[E0282]: type annotations needed
    --> compiler/rustc_middle/src/ty/mod.rs:1786:21
    --> compiler/rustc_middle/src/ty/mod.rs:1786:21
     |
1786 |         for attr in tcx.get_attrs(did, sym::repr) {

error[E0282]: type annotations needed
    --> compiler/rustc_middle/src/ty/mod.rs:2171:35
     |
     |
2171 |         self.get_attrs(did, attr).next()
     |
help: try using a fully qualified path to specify the expected types
     |
     |
2171 |         <Self as Iterator>::next(&mut self.get_attrs(did, attr))

error[E0282]: type annotations needed
    --> compiler/rustc_middle/src/ty/mod.rs:2179:39
     |
     |
2179 |             self.get_attrs(did, attr).next().is_some()
     |
help: try using a fully qualified path to specify the expected types
     |
     |
2179 |             <Self as Iterator>::next(&mut self.get_attrs(did, attr)).is_some()

For more information about this error, try `rustc --explain E0282`.
error: could not compile `rustc_middle` due to 4 previous errors
warning: build failed, waiting for other jobs to finish...