diff --git a/compiler/rustc_trait_selection/src/traits/auto_trait.rs b/compiler/rustc_trait_selection/src/traits/auto_trait.rs index c909a0b49e24e..7603c9ed7a862 100644 --- a/compiler/rustc_trait_selection/src/traits/auto_trait.rs +++ b/compiler/rustc_trait_selection/src/traits/auto_trait.rs @@ -6,13 +6,13 @@ use super::*; use crate::errors::UnableToConstructConstantValue; use crate::infer::region_constraints::{Constraint, RegionConstraintData}; use crate::traits::project::ProjectAndUnifyResult; + +use rustc_data_structures::fx::{FxIndexMap, FxIndexSet, IndexEntry}; +use rustc_data_structures::unord::UnordSet; use rustc_infer::infer::DefineOpaqueTypes; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::ty::{Region, RegionVid}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; - -use std::collections::hash_map::Entry; use std::collections::VecDeque; use std::iter; @@ -35,17 +35,10 @@ pub enum AutoTraitResult { NegativeImpl, } -#[allow(dead_code)] -impl AutoTraitResult { - fn is_auto(&self) -> bool { - matches!(self, AutoTraitResult::PositiveImpl(_) | AutoTraitResult::NegativeImpl) - } -} - pub struct AutoTraitInfo<'cx> { pub full_user_env: ty::ParamEnv<'cx>, pub region_data: RegionConstraintData<'cx>, - pub vid_to_region: FxHashMap>, + pub vid_to_region: FxIndexMap>, } pub struct AutoTraitFinder<'tcx> { @@ -114,7 +107,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { } let infcx = tcx.infer_ctxt().build(); - let mut fresh_preds = FxHashSet::default(); + let mut fresh_preds = FxIndexSet::default(); // Due to the way projections are handled by SelectionContext, we need to run // evaluate_predicates twice: once on the original param env, and once on the result of @@ -239,7 +232,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { ty: Ty<'tcx>, param_env: ty::ParamEnv<'tcx>, user_env: ty::ParamEnv<'tcx>, - fresh_preds: &mut FxHashSet>, + fresh_preds: &mut FxIndexSet>, ) -> Option<(ty::ParamEnv<'tcx>, ty::ParamEnv<'tcx>)> { let tcx = infcx.tcx; @@ -252,7 +245,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { let mut select = SelectionContext::new(infcx); - let mut already_visited = FxHashSet::default(); + let mut already_visited = UnordSet::new(); let mut predicates = VecDeque::new(); predicates.push_back(ty::Binder::dummy(ty::TraitPredicate { trait_ref: ty::TraitRef::new(infcx.tcx, trait_did, [ty]), @@ -473,9 +466,9 @@ impl<'tcx> AutoTraitFinder<'tcx> { fn map_vid_to_region<'cx>( &self, regions: &RegionConstraintData<'cx>, - ) -> FxHashMap> { - let mut vid_map: FxHashMap, RegionDeps<'cx>> = FxHashMap::default(); - let mut finished_map = FxHashMap::default(); + ) -> FxIndexMap> { + let mut vid_map = FxIndexMap::, RegionDeps<'cx>>::default(); + let mut finished_map = FxIndexMap::default(); for (constraint, _) in ®ions.constraints { match constraint { @@ -513,25 +506,22 @@ impl<'tcx> AutoTraitFinder<'tcx> { } while !vid_map.is_empty() { - #[allow(rustc::potential_query_instability)] - let target = *vid_map.keys().next().expect("Keys somehow empty"); - let deps = vid_map.remove(&target).expect("Entry somehow missing"); + let target = *vid_map.keys().next().unwrap(); + let deps = vid_map.swap_remove(&target).unwrap(); for smaller in deps.smaller.iter() { for larger in deps.larger.iter() { match (smaller, larger) { (&RegionTarget::Region(_), &RegionTarget::Region(_)) => { - if let Entry::Occupied(v) = vid_map.entry(*smaller) { + if let IndexEntry::Occupied(v) = vid_map.entry(*smaller) { let smaller_deps = v.into_mut(); smaller_deps.larger.insert(*larger); - // FIXME(#120456) - is `swap_remove` correct? smaller_deps.larger.swap_remove(&target); } - if let Entry::Occupied(v) = vid_map.entry(*larger) { + if let IndexEntry::Occupied(v) = vid_map.entry(*larger) { let larger_deps = v.into_mut(); larger_deps.smaller.insert(*smaller); - // FIXME(#120456) - is `swap_remove` correct? larger_deps.smaller.swap_remove(&target); } } @@ -542,17 +532,15 @@ impl<'tcx> AutoTraitFinder<'tcx> { // Do nothing; we don't care about regions that are smaller than vids. } (&RegionTarget::RegionVid(_), &RegionTarget::RegionVid(_)) => { - if let Entry::Occupied(v) = vid_map.entry(*smaller) { + if let IndexEntry::Occupied(v) = vid_map.entry(*smaller) { let smaller_deps = v.into_mut(); smaller_deps.larger.insert(*larger); - // FIXME(#120456) - is `swap_remove` correct? smaller_deps.larger.swap_remove(&target); } - if let Entry::Occupied(v) = vid_map.entry(*larger) { + if let IndexEntry::Occupied(v) = vid_map.entry(*larger) { let larger_deps = v.into_mut(); larger_deps.smaller.insert(*smaller); - // FIXME(#120456) - is `swap_remove` correct? larger_deps.smaller.swap_remove(&target); } } @@ -560,6 +548,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { } } } + finished_map } @@ -588,7 +577,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { ty: Ty<'_>, nested: impl Iterator>, computed_preds: &mut FxIndexSet>, - fresh_preds: &mut FxHashSet>, + fresh_preds: &mut FxIndexSet>, predicates: &mut VecDeque>, selcx: &mut SelectionContext<'_, 'tcx>, ) -> bool { diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index fbc2c3c5af459..acac686a6fca6 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -15,18 +15,15 @@ enum RegionTarget<'tcx> { #[derive(Default, Debug, Clone)] struct RegionDeps<'tcx> { - larger: FxHashSet>, - smaller: FxHashSet>, + larger: FxIndexSet>, + smaller: FxIndexSet>, } pub(crate) struct AutoTraitFinder<'a, 'tcx> { pub(crate) cx: &'a mut core::DocContext<'tcx>, } -impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> -where - 'tcx: 'a, // should be an implied bound; rustc bug #98852. -{ +impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> { pub(crate) fn new(cx: &'a mut core::DocContext<'tcx>) -> Self { AutoTraitFinder { cx } } @@ -158,7 +155,7 @@ where auto_traits } - fn get_lifetime(region: Region<'_>, names_map: &FxHashMap) -> Lifetime { + fn get_lifetime(region: Region<'_>, names_map: &FxIndexMap) -> Lifetime { region_name(region) .map(|name| { names_map @@ -181,15 +178,15 @@ where /// existing inference/solver code do what we want. fn handle_lifetimes<'cx>( regions: &RegionConstraintData<'cx>, - names_map: &FxHashMap, + names_map: &FxIndexMap, ) -> ThinVec { // Our goal is to 'flatten' the list of constraints by eliminating // all intermediate RegionVids. At the end, all constraints should // be between Regions (aka region variables). This gives us the information // we need to create the Generics. - let mut finished: FxHashMap<_, Vec<_>> = Default::default(); + let mut finished: FxIndexMap<_, Vec<_>> = Default::default(); - let mut vid_map: FxHashMap, RegionDeps<'_>> = Default::default(); + let mut vid_map: FxIndexMap, RegionDeps<'_>> = Default::default(); // Flattening is done in two parts. First, we insert all of the constraints // into a map. Each RegionTarget (either a RegionVid or a Region) maps @@ -245,8 +242,8 @@ where // constraint, and add it to our list. Since we make sure to never re-add // deleted items, this process will always finish. while !vid_map.is_empty() { - let target = *vid_map.keys().next().expect("Keys somehow empty"); - let deps = vid_map.remove(&target).expect("Entry somehow missing"); + let target = *vid_map.keys().next().unwrap(); + let deps = vid_map.swap_remove(&target).unwrap(); for smaller in deps.smaller.iter() { for larger in deps.larger.iter() { @@ -260,30 +257,30 @@ where } } (&RegionTarget::RegionVid(_), &RegionTarget::Region(_)) => { - if let Entry::Occupied(v) = vid_map.entry(*smaller) { + if let IndexEntry::Occupied(v) = vid_map.entry(*smaller) { let smaller_deps = v.into_mut(); smaller_deps.larger.insert(*larger); - smaller_deps.larger.remove(&target); + smaller_deps.larger.swap_remove(&target); } } (&RegionTarget::Region(_), &RegionTarget::RegionVid(_)) => { - if let Entry::Occupied(v) = vid_map.entry(*larger) { + if let IndexEntry::Occupied(v) = vid_map.entry(*larger) { let deps = v.into_mut(); deps.smaller.insert(*smaller); - deps.smaller.remove(&target); + deps.smaller.swap_remove(&target); } } (&RegionTarget::RegionVid(_), &RegionTarget::RegionVid(_)) => { - if let Entry::Occupied(v) = vid_map.entry(*smaller) { + if let IndexEntry::Occupied(v) = vid_map.entry(*smaller) { let smaller_deps = v.into_mut(); smaller_deps.larger.insert(*larger); - smaller_deps.larger.remove(&target); + smaller_deps.larger.swap_remove(&target); } - if let Entry::Occupied(v) = vid_map.entry(*larger) { + if let IndexEntry::Occupied(v) = vid_map.entry(*larger) { let larger_deps = v.into_mut(); larger_deps.smaller.insert(*smaller); - larger_deps.smaller.remove(&target); + larger_deps.smaller.swap_remove(&target); } } } @@ -295,7 +292,7 @@ where .iter() .flat_map(|(name, lifetime)| { let empty = Vec::new(); - let bounds: FxHashSet = finished + let bounds: FxIndexSet = finished .get(name) .unwrap_or(&empty) .iter() @@ -315,7 +312,7 @@ where lifetime_predicates } - fn extract_for_generics(&self, pred: ty::Clause<'tcx>) -> FxHashSet { + fn extract_for_generics(&self, pred: ty::Clause<'tcx>) -> FxIndexSet { let bound_predicate = pred.kind(); let tcx = self.cx.tcx; let regions = @@ -324,7 +321,7 @@ where .collect_referenced_late_bound_regions(bound_predicate.rebind(poly_trait_pred)), ty::ClauseKind::Projection(poly_proj_pred) => tcx .collect_referenced_late_bound_regions(bound_predicate.rebind(poly_proj_pred)), - _ => return FxHashSet::default(), + _ => return FxIndexSet::default(), }; regions @@ -342,9 +339,9 @@ where fn make_final_bounds( &self, - ty_to_bounds: FxHashMap>, - ty_to_fn: FxHashMap)>, - lifetime_to_bounds: FxHashMap>, + ty_to_bounds: FxIndexMap>, + ty_to_fn: FxIndexMap)>, + lifetime_to_bounds: FxIndexMap>, ) -> Vec { ty_to_bounds .into_iter() @@ -390,20 +387,16 @@ where return None; } - let mut bounds_vec = bounds.into_iter().collect(); - self.sort_where_bounds(&mut bounds_vec); - Some(WherePredicate::BoundPredicate { ty, - bounds: bounds_vec, + bounds: bounds.into_iter().collect(), bound_params: Vec::new(), }) }) .chain(lifetime_to_bounds.into_iter().filter(|(_, bounds)| !bounds.is_empty()).map( - |(lifetime, bounds)| { - let mut bounds_vec = bounds.into_iter().collect(); - self.sort_where_bounds(&mut bounds_vec); - WherePredicate::RegionPredicate { lifetime, bounds: bounds_vec } + |(lifetime, bounds)| WherePredicate::RegionPredicate { + lifetime, + bounds: bounds.into_iter().collect(), }, )) .collect() @@ -418,19 +411,14 @@ where /// * `Fn` bounds are handled specially - instead of leaving it as `T: Fn(), = /// K`, we use the dedicated syntax `T: Fn() -> K` /// * We explicitly add a `?Sized` bound if we didn't find any `Sized` predicates for a type + #[instrument(level = "debug", skip(self, vid_to_region))] fn param_env_to_generics( &mut self, item_def_id: DefId, param_env: ty::ParamEnv<'tcx>, mut existing_predicates: ThinVec, - vid_to_region: FxHashMap>, + vid_to_region: FxIndexMap>, ) -> Generics { - debug!( - "param_env_to_generics(item_def_id={:?}, param_env={:?}, \ - existing_predicates={:?})", - item_def_id, param_env, existing_predicates - ); - let tcx = self.cx.tcx; // The `Sized` trait must be handled specially, since we only display it when @@ -439,6 +427,7 @@ where let mut replacer = RegionReplacer { vid_to_region: &vid_to_region, tcx }; + // FIXME(fmease): Remove this! let orig_bounds: FxHashSet<_> = tcx.param_env(item_def_id).caller_bounds().iter().collect(); let clean_where_predicates = param_env .caller_bounds() @@ -461,12 +450,11 @@ where debug!("param_env_to_generics({item_def_id:?}): generic_params={generic_params:?}"); - let mut has_sized = FxHashSet::default(); - let mut ty_to_bounds: FxHashMap<_, FxHashSet<_>> = Default::default(); - let mut lifetime_to_bounds: FxHashMap<_, FxHashSet<_>> = Default::default(); - let mut ty_to_traits: FxHashMap> = Default::default(); - - let mut ty_to_fn: FxHashMap)> = Default::default(); + let mut has_sized = FxHashSet::default(); // NOTE(fmease): not used for iteration + let mut ty_to_bounds = FxIndexMap::<_, FxIndexSet<_>>::default(); + let mut lifetime_to_bounds = FxIndexMap::<_, FxIndexSet<_>>::default(); + let mut ty_to_traits = FxIndexMap::>::default(); + let mut ty_to_fn = FxIndexMap::)>::default(); // FIXME: This code shares much of the logic found in `clean_ty_generics` and // `simplify::where_clause`. Consider deduplicating it to avoid diverging @@ -526,7 +514,7 @@ where // that we don't end up with duplicate bounds (e.g., for<'b, 'b>) for_generics.extend(p.generic_params.drain(..)); p.generic_params.extend(for_generics); - self.is_fn_trait(&p.trait_) + tcx.is_fn_trait(p.trait_.def_id()) } _ => false, }; @@ -559,7 +547,7 @@ where let ty = &*self_type; let mut new_trait = trait_.clone(); - if self.is_fn_trait(trait_) && assoc.name == sym::Output { + if tcx.is_fn_trait(trait_.def_id()) && assoc.name == sym::Output { ty_to_fn .entry(ty.clone()) .and_modify(|e| { @@ -611,7 +599,7 @@ where // that we don't see a // duplicate bound like `T: Iterator + Iterator` // on the docs page. - bounds.remove(&GenericBound::TraitBound( + bounds.swap_remove(&GenericBound::TraitBound( PolyTrait { trait_: trait_.clone(), generic_params: Vec::new() }, hir::TraitBoundModifier::None, )); @@ -647,75 +635,8 @@ where } } - self.sort_where_predicates(&mut existing_predicates); - Generics { params: generic_params, where_predicates: existing_predicates } } - - /// Ensure that the predicates are in a consistent order. The precise - /// ordering doesn't actually matter, but it's important that - /// a given set of predicates always appears in the same order - - /// both for visual consistency between 'rustdoc' runs, and to - /// make writing tests much easier - #[inline] - fn sort_where_predicates(&self, predicates: &mut [WherePredicate]) { - // We should never have identical bounds - and if we do, - // they're visually identical as well. Therefore, using - // an unstable sort is fine. - self.unstable_debug_sort(predicates); - } - - /// Ensure that the bounds are in a consistent order. The precise - /// ordering doesn't actually matter, but it's important that - /// a given set of bounds always appears in the same order - - /// both for visual consistency between 'rustdoc' runs, and to - /// make writing tests much easier - #[inline] - fn sort_where_bounds(&self, bounds: &mut Vec) { - // We should never have identical bounds - and if we do, - // they're visually identical as well. Therefore, using - // an unstable sort is fine. - self.unstable_debug_sort(bounds); - } - - /// This might look horrendously hacky, but it's actually not that bad. - /// - /// For performance reasons, we use several different FxHashMaps - /// in the process of computing the final set of where predicates. - /// However, the iteration order of a HashMap is completely unspecified. - /// In fact, the iteration of an FxHashMap can even vary between platforms, - /// since FxHasher has different behavior for 32-bit and 64-bit platforms. - /// - /// Obviously, it's extremely undesirable for documentation rendering - /// to be dependent on the platform it's run on. Apart from being confusing - /// to end users, it makes writing tests much more difficult, as predicates - /// can appear in any order in the final result. - /// - /// To solve this problem, we sort WherePredicates and GenericBounds - /// by their Debug string. The thing to keep in mind is that we don't really - /// care what the final order is - we're synthesizing an impl or bound - /// ourselves, so any order can be considered equally valid. By sorting the - /// predicates and bounds, however, we ensure that for a given codebase, all - /// auto-trait impls always render in exactly the same way. - /// - /// Using the Debug implementation for sorting prevents us from needing to - /// write quite a bit of almost entirely useless code (e.g., how should two - /// Types be sorted relative to each other). It also allows us to solve the - /// problem for both WherePredicates and GenericBounds at the same time. This - /// approach is probably somewhat slower, but the small number of items - /// involved (impls rarely have more than a few bounds) means that it - /// shouldn't matter in practice. - fn unstable_debug_sort(&self, vec: &mut [T]) { - vec.sort_by_cached_key(|x| format!("{x:?}")) - } - - fn is_fn_trait(&self, path: &Path) -> bool { - let tcx = self.cx.tcx; - let did = path.def_id(); - did == tcx.require_lang_item(LangItem::Fn, None) - || did == tcx.require_lang_item(LangItem::FnMut, None) - || did == tcx.require_lang_item(LangItem::FnOnce, None) - } } fn region_name(region: Region<'_>) -> Option { @@ -727,7 +648,7 @@ fn region_name(region: Region<'_>) -> Option { /// Replaces all [`ty::RegionVid`]s in a type with [`ty::Region`]s, using the provided map. struct RegionReplacer<'a, 'tcx> { - vid_to_region: &'a FxHashMap>, + vid_to_region: &'a FxIndexMap>, tcx: TyCtxt<'tcx>, } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 0cdf52bfb0046..9023d9d411791 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -35,7 +35,6 @@ use rustc_span::{self, ExpnKind}; use rustc_trait_selection::traits::wf::object_region_bounds; use std::borrow::Cow; -use std::collections::hash_map::Entry; use std::collections::BTreeMap; use std::hash::Hash; use std::mem; diff --git a/tests/rustdoc/synthetic_auto/complex.rs b/tests/rustdoc/synthetic_auto/complex.rs index 4c39f0bf1e07f..21055287c9963 100644 --- a/tests/rustdoc/synthetic_auto/complex.rs +++ b/tests/rustdoc/synthetic_auto/complex.rs @@ -21,8 +21,8 @@ mod foo { // @has complex/struct.NotOuter.html // @has - '//*[@id="synthetic-implementations-list"]//*[@class="impl"]//h3[@class="code-header"]' \ -// "impl<'a, T, K: ?Sized> Send for Outer<'a, T, K>where K: for<'b> Fn((&'b bool, &'a u8)) \ -// -> &'b i8, T: MyTrait<'a>, >::MyItem: Copy, 'a: 'static" +// "impl<'a, T, K: ?Sized> Send for Outer<'a, T, K>where 'a: 'static, T: MyTrait<'a>, \ +// K: for<'b> Fn((&'b bool, &'a u8)) -> &'b i8, >::MyItem: Copy," pub use foo::{Foo, Inner as NotInner, MyTrait as NotMyTrait, Outer as NotOuter}; diff --git a/tests/rustdoc/synthetic_auto/lifetimes.rs b/tests/rustdoc/synthetic_auto/lifetimes.rs index 71265b3078a06..23e1efdaeef19 100644 --- a/tests/rustdoc/synthetic_auto/lifetimes.rs +++ b/tests/rustdoc/synthetic_auto/lifetimes.rs @@ -10,7 +10,7 @@ where // @has lifetimes/struct.Foo.html // @has - '//*[@id="synthetic-implementations-list"]//*[@class="impl"]//h3[@class="code-header"]' \ -// "impl<'c, K> Send for Foo<'c, K>where K: for<'b> Fn(&'b bool) -> &'c u8, 'c: 'static" +// "impl<'c, K> Send for Foo<'c, K>where 'c: 'static, K: for<'b> Fn(&'b bool) -> &'c u8," // // @has - '//*[@id="synthetic-implementations-list"]//*[@class="impl"]//h3[@class="code-header"]' \ // "impl<'c, K> Sync for Foo<'c, K>where K: Sync" diff --git a/tests/rustdoc/synthetic_auto/no-redundancy.rs b/tests/rustdoc/synthetic_auto/no-redundancy.rs index d30b38dd4dc1d..64dab429647ae 100644 --- a/tests/rustdoc/synthetic_auto/no-redundancy.rs +++ b/tests/rustdoc/synthetic_auto/no-redundancy.rs @@ -1,6 +1,3 @@ -// FIXME(fmease, #119216): Reenable this test! -//@ ignore-test - pub struct Inner { field: T, } @@ -13,7 +10,7 @@ where // @has no_redundancy/struct.Outer.html // @has - '//*[@id="synthetic-implementations-list"]//*[@class="impl"]//h3[@class="code-header"]' \ -// "impl Send for Outerwhere T: Send + Copy" +// "impl Send for Outerwhere T: Copy + Send" pub struct Outer { inner_field: Inner, } diff --git a/tests/rustdoc/synthetic_auto/project.rs b/tests/rustdoc/synthetic_auto/project.rs index 7c9412ae96243..f4ede76e6debf 100644 --- a/tests/rustdoc/synthetic_auto/project.rs +++ b/tests/rustdoc/synthetic_auto/project.rs @@ -24,11 +24,11 @@ where // @has project/struct.Foo.html // @has - '//*[@id="synthetic-implementations-list"]//*[@class="impl"]//h3[@class="code-header"]' \ -// "impl<'c, K> Send for Foo<'c, K>where K: MyTrait, 'c: 'static" +// "impl<'c, K> Send for Foo<'c, K>where 'c: 'static, K: MyTrait," // // @has - '//*[@id="synthetic-implementations-list"]//*[@class="impl"]//h3[@class="code-header"]' \ -// "impl<'c, K> Sync for Foo<'c, K>where K: MyTrait, ::MyItem: OtherTrait, \ -// 'c: 'static," +// "impl<'c, K> Sync for Foo<'c, K>where 'c: 'static, K: MyTrait, \ +// ::MyItem: OtherTrait," pub struct Foo<'c, K: 'c> { inner_field: Inner<'c, K>, }