Skip to content

Commit

Permalink
Auto merge of #89408 - Mark-Simulacrum:fix-query-nondet, r=petrochenkov
Browse files Browse the repository at this point in the history
Avoid nondeterminism in trimmed_def_paths

Previously this query depended on the global interning order of Symbols, which
meant that irrelevant changes could influence the query and cause
recompilations. This commit ensures that the return set is stable and will not
be affected by the global order by deterministically (in lexicographic order)
choosing a name to use if there are multiple names for a single DefId.

This should fix the cause of the [regressions] in #83343.

[regressions]: https://perf.rust-lang.org/compare.html?start=9620f3a84b079decfdc2e557be007580b097fe43&end=addb4da686a97da46159f0123cb6cdc2ce3d7fdb
  • Loading branch information
bors committed Oct 2, 2021
2 parents b27661e + 56fcf07 commit edebf77
Showing 1 changed file with 23 additions and 2 deletions.
25 changes: 23 additions & 2 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2408,7 +2408,7 @@ fn for_each_def(tcx: TyCtxt<'_>, mut collect_fn: impl for<'b> FnMut(&'b Ident, N
///
/// The implementation uses similar import discovery logic to that of 'use' suggestions.
fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> FxHashMap<DefId, Symbol> {
let mut map = FxHashMap::default();
let mut map: FxHashMap<DefId, Symbol> = FxHashMap::default();

if let TrimmedDefPaths::GoodPath = tcx.sess.opts.trimmed_def_paths {
// For good paths causing this bug, the `rustc_middle::ty::print::with_no_trimmed_paths`
Expand Down Expand Up @@ -2446,8 +2446,29 @@ fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> FxHashMap<DefId, Symbol> {
});

for ((_, symbol), opt_def_id) in unique_symbols_rev.drain() {
use std::collections::hash_map::Entry::{Occupied, Vacant};

if let Some(def_id) = opt_def_id {
map.insert(def_id, symbol);
match map.entry(def_id) {
Occupied(mut v) => {
// A single DefId can be known under multiple names (e.g.,
// with a `pub use ... as ...;`). We need to ensure that the
// name placed in this map is chosen deterministically, so
// if we find multiple names (`symbol`) resolving to the
// same `def_id`, we prefer the lexicographically smallest
// name.
//
// Any stable ordering would be fine here though.
if *v.get() != symbol {
if v.get().as_str() > symbol.as_str() {
v.insert(symbol);
}
}
}
Vacant(v) => {
v.insert(symbol);
}
}
}
}

Expand Down

0 comments on commit edebf77

Please sign in to comment.