Auto merge of rust-lang#113328 - michaelwoerister:no_hashmap_in_typeck, r=cjgillot,lcnr

Enable potential_query_instability lint in rustc_hir_typeck.

Fix linting errors by using `FxIndex(Map|Set)` and `Unord(Map|Set)` as appropriate. Part of [MCP 533](rust-lang/compiler-team#533).

I really like the `potential_query_instability` lint!

r? `@lcnr`

Author: bors

compiler/rustc_data_structures/src/unord.rs

@@ -31,6 +31,7 @@ use crate::{
 ///
 /// It's still possible to do the same thing with an `Fn` by using interior mutability,
 /// but the chance of doing it accidentally is reduced.
+#[derive(Clone)]
 pub struct UnordItems<T, I: Iterator<Item = T>>(I);
 
 impl<T, I: Iterator<Item = T>> UnordItems<T, I> {
@@ -167,6 +168,14 @@ impl<T: Ord, I: Iterator<Item = T>> UnordItems<T, I> {
     }
 }
 
+/// A marker trait specifying that `Self` can consume `UnordItems<_>` without
+/// exposing any internal ordering.
+///
+/// Note: right now this is just a marker trait. It could be extended to contain
+/// some useful, common methods though, like `len`, `clear`, or the various
+/// kinds of `to_sorted`.
+trait UnordCollection {}
+
 /// This is a set collection type that tries very hard to not expose
 /// any internal iteration. This is a useful property when trying to
 /// uphold the determinism invariants imposed by the query system.
@@ -181,6 +190,8 @@ pub struct UnordSet<V: Eq + Hash> {
     inner: FxHashSet<V>,
 }
 
+impl<V: Eq + Hash> UnordCollection for UnordSet<V> {}
+
 impl<V: Eq + Hash> Default for UnordSet<V> {
     #[inline]
     fn default() -> Self {
@@ -194,6 +205,11 @@ impl<V: Eq + Hash> UnordSet<V> {
         Self { inner: Default::default() }
     }
 
+    #[inline]
+    pub fn with_capacity(capacity: usize) -> Self {
+        Self { inner: FxHashSet::with_capacity_and_hasher(capacity, Default::default()) }
+    }
+
     #[inline]
     pub fn len(&self) -> usize {
         self.inner.len()
@@ -258,9 +274,9 @@ impl<V: Eq + Hash> UnordSet<V> {
     #[inline]
     pub fn to_sorted_stable_ord(&self) -> Vec<V>
     where
-        V: Ord + StableOrd + Copy,
+        V: Ord + StableOrd + Clone,
     {
-        let mut items: Vec<V> = self.inner.iter().copied().collect();
+        let mut items: Vec<V> = self.inner.iter().cloned().collect();
         items.sort_unstable();
         items
     }
@@ -279,16 +295,28 @@ impl<V: Eq + Hash> UnordSet<V> {
         to_sorted_vec(hcx, self.inner.into_iter(), cache_sort_key, |x| x)
     }
 
-    // We can safely extend this UnordSet from a set of unordered values because that
-    // won't expose the internal ordering anywhere.
     #[inline]
-    pub fn extend_unord<I: Iterator<Item = V>>(&mut self, items: UnordItems<V, I>) {
-        self.inner.extend(items.0)
+    pub fn clear(&mut self) {
+        self.inner.clear();
     }
+}
+
+pub trait ExtendUnord<T> {
+    /// Extend this unord collection with the given `UnordItems`.
+    /// This method is called `extend_unord` instead of just `extend` so it
+    /// does not conflict with `Extend::extend`. Otherwise there would be many
+    /// places where the two methods would have to be explicitly disambiguated
+    /// via UFCS.
+    fn extend_unord<I: Iterator<Item = T>>(&mut self, items: UnordItems<T, I>);
+}
 
+// Note: it is important that `C` implements `UnordCollection` in addition to
+// `Extend`, otherwise this impl would leak the internal iteration order of
+// `items`, e.g. when calling `some_vec.extend_unord(some_unord_items)`.
+impl<C: Extend<T> + UnordCollection, T> ExtendUnord<T> for C {
     #[inline]
-    pub fn clear(&mut self) {
-        self.inner.clear();
+    fn extend_unord<I: Iterator<Item = T>>(&mut self, items: UnordItems<T, I>) {
+        self.extend(items.0)
     }
 }
 
@@ -312,6 +340,12 @@ impl<V: Hash + Eq> From<FxHashSet<V>> for UnordSet<V> {
     }
 }
 
+impl<V: Hash + Eq, I: Iterator<Item = V>> From<UnordItems<V, I>> for UnordSet<V> {
+    fn from(value: UnordItems<V, I>) -> Self {
+        UnordSet { inner: FxHashSet::from_iter(value.0) }
+    }
+}
+
 impl<HCX, V: Hash + Eq + HashStable<HCX>> HashStable<HCX> for UnordSet<V> {
     #[inline]
     fn hash_stable(&self, hcx: &mut HCX, hasher: &mut StableHasher) {
@@ -333,6 +367,8 @@ pub struct UnordMap<K: Eq + Hash, V> {
     inner: FxHashMap<K, V>,
 }
 
+impl<K: Eq + Hash, V> UnordCollection for UnordMap<K, V> {}
+
 impl<K: Eq + Hash, V> Default for UnordMap<K, V> {
     #[inline]
     fn default() -> Self {
@@ -362,6 +398,11 @@ impl<K: Hash + Eq, V, I: Iterator<Item = (K, V)>> From<UnordItems<(K, V), I>> fo
 }
 
 impl<K: Eq + Hash, V> UnordMap<K, V> {
+    #[inline]
+    pub fn with_capacity(capacity: usize) -> Self {
+        Self { inner: FxHashMap::with_capacity_and_hasher(capacity, Default::default()) }
+    }
+
     #[inline]
     pub fn len(&self) -> usize {
         self.inner.len()
@@ -428,13 +469,6 @@ impl<K: Eq + Hash, V> UnordMap<K, V> {
         UnordItems(self.inner.into_iter())
     }
 
-    // We can safely extend this UnordMap from a set of unordered values because that
-    // won't expose the internal ordering anywhere.
-    #[inline]
-    pub fn extend<I: Iterator<Item = (K, V)>>(&mut self, items: UnordItems<(K, V), I>) {
-        self.inner.extend(items.0)
-    }
-
     /// Returns the entries of this map in stable sort order (as defined by `ToStableHashKey`).
     ///
     /// The `cache_sort_key` parameter controls if [slice::sort_by_cached_key] or
@@ -554,15 +588,10 @@ impl<V> UnordBag<V> {
     pub fn into_items(self) -> UnordItems<V, impl Iterator<Item = V>> {
         UnordItems(self.inner.into_iter())
     }
-
-    // We can safely extend this UnordSet from a set of unordered values because that
-    // won't expose the internal ordering anywhere.
-    #[inline]
-    pub fn extend<I: Iterator<Item = V>>(&mut self, items: UnordItems<V, I>) {
-        self.inner.extend(items.0)
-    }
 }
 
+impl<T> UnordCollection for UnordBag<T> {}
+
 impl<T> Extend<T> for UnordBag<T> {
     fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
         self.inner.extend(iter)

compiler/rustc_hir_analysis/src/check_unused.rs

@@ -1,4 +1,4 @@
-use rustc_data_structures::unord::UnordSet;
+use rustc_data_structures::unord::{ExtendUnord, UnordSet};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::LocalDefId;
 use rustc_middle::ty::TyCtxt;

compiler/rustc_hir_typeck/src/fallback.rs

@@ -1,8 +1,8 @@
 use crate::FnCtxt;
 use rustc_data_structures::{
-    fx::{FxHashMap, FxHashSet},
     graph::WithSuccessors,
     graph::{iterate::DepthFirstSearch, vec_graph::VecGraph},
+    unord::{UnordBag, UnordMap, UnordSet},
 };
 use rustc_middle::ty::{self, Ty};
 
@@ -83,7 +83,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
     fn fallback_if_possible(
         &self,
         ty: Ty<'tcx>,
-        diverging_fallback: &FxHashMap<Ty<'tcx>, Ty<'tcx>>,
+        diverging_fallback: &UnordMap<Ty<'tcx>, Ty<'tcx>>,
     ) {
         // Careful: we do NOT shallow-resolve `ty`. We know that `ty`
         // is an unsolved variable, and we determine its fallback
@@ -193,7 +193,7 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
     fn calculate_diverging_fallback(
         &self,
         unsolved_variables: &[Ty<'tcx>],
-    ) -> FxHashMap<Ty<'tcx>, Ty<'tcx>> {
+    ) -> UnordMap<Ty<'tcx>, Ty<'tcx>> {
         debug!("calculate_diverging_fallback({:?})", unsolved_variables);
 
         // Construct a coercion graph where an edge `A -> B` indicates
@@ -210,10 +210,10 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
         //
         // These variables are the ones that are targets for fallback to
         // either `!` or `()`.
-        let diverging_roots: FxHashSet<ty::TyVid> = self
+        let diverging_roots: UnordSet<ty::TyVid> = self
             .diverging_type_vars
             .borrow()
-            .iter()
+            .items()
             .map(|&ty| self.shallow_resolve(ty))
             .filter_map(|ty| ty.ty_vid())
             .map(|vid| self.root_var(vid))
@@ -284,23 +284,27 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
         // For each diverging variable, figure out whether it can
         // reach a member of N. If so, it falls back to `()`. Else
         // `!`.
-        let mut diverging_fallback = FxHashMap::default();
-        diverging_fallback.reserve(diverging_vids.len());
+        let mut diverging_fallback = UnordMap::with_capacity(diverging_vids.len());
         for &diverging_vid in &diverging_vids {
             let diverging_ty = Ty::new_var(self.tcx, diverging_vid);
             let root_vid = self.root_var(diverging_vid);
             let can_reach_non_diverging = coercion_graph
                 .depth_first_search(root_vid)
                 .any(|n| roots_reachable_from_non_diverging.visited(n));
 
-            let mut found_infer_var_info = ty::InferVarInfo { self_in_trait: false, output: false };
+            let infer_var_infos: UnordBag<_> = self
+                .inh
+                .infer_var_info
+                .borrow()
+                .items()
+                .filter(|&(vid, _)| self.infcx.root_var(*vid) == root_vid)
+                .map(|(_, info)| *info)
+                .collect();
 
-            for (vid, info) in self.inh.infer_var_info.borrow().iter() {
-                if self.infcx.root_var(*vid) == root_vid {
-                    found_infer_var_info.self_in_trait |= info.self_in_trait;
-                    found_infer_var_info.output |= info.output;
-                }
-            }
+            let found_infer_var_info = ty::InferVarInfo {
+                self_in_trait: infer_var_infos.items().any(|info| info.self_in_trait),
+                output: infer_var_infos.items().any(|info| info.output),
+            };
 
             if found_infer_var_info.self_in_trait && found_infer_var_info.output {
                 // This case falls back to () to ensure that the code pattern in

compiler/rustc_hir_typeck/src/generator_interior/drop_ranges/cfg_build.rs

@@ -6,7 +6,7 @@ use hir::{
     intravisit::{self, Visitor},
     Body, Expr, ExprKind, Guard, HirId, LoopIdError,
 };
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_hir as hir;
 use rustc_index::IndexVec;
 use rustc_infer::infer::InferCtxt;
@@ -28,7 +28,7 @@ pub(super) fn build_control_flow_graph<'tcx>(
     consumed_borrowed_places: ConsumedAndBorrowedPlaces,
     body: &'tcx Body<'tcx>,
     num_exprs: usize,
-) -> (DropRangesBuilder, FxHashSet<HirId>) {
+) -> (DropRangesBuilder, UnordSet<HirId>) {
     let mut drop_range_visitor = DropRangeVisitor::new(
         infcx,
         typeck_results,
@@ -528,7 +528,7 @@ impl DropRangesBuilder {
         hir: Map<'_>,
         num_exprs: usize,
     ) -> Self {
-        let mut tracked_value_map = FxHashMap::<_, TrackedValueIndex>::default();
+        let mut tracked_value_map = UnordMap::<_, TrackedValueIndex>::default();
         let mut next = <_>::from(0u32);
         for value in tracked_values {
             for_each_consumable(hir, value, |value| {

compiler/rustc_hir_typeck/src/generator_interior/drop_ranges/mod.rs

@@ -17,7 +17,7 @@ use self::record_consumed_borrow::find_consumed_and_borrowed;
 use crate::FnCtxt;
 use hir::def_id::DefId;
 use hir::{Body, HirId, HirIdMap, Node};
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_hir as hir;
 use rustc_index::bit_set::BitSet;
 use rustc_index::IndexVec;
@@ -63,7 +63,7 @@ pub fn compute_drop_ranges<'a, 'tcx>(
         // If drop range tracking is not enabled, skip all the analysis and produce an
         // empty set of DropRanges.
         DropRanges {
-            tracked_value_map: FxHashMap::default(),
+            tracked_value_map: UnordMap::default(),
             nodes: IndexVec::new(),
             borrowed_temporaries: None,
         }
@@ -182,9 +182,9 @@ impl TryFrom<&PlaceWithHirId<'_>> for TrackedValue {
 }
 
 pub struct DropRanges {
-    tracked_value_map: FxHashMap<TrackedValue, TrackedValueIndex>,
+    tracked_value_map: UnordMap<TrackedValue, TrackedValueIndex>,
     nodes: IndexVec<PostOrderId, NodeInfo>,
-    borrowed_temporaries: Option<FxHashSet<HirId>>,
+    borrowed_temporaries: Option<UnordSet<HirId>>,
 }
 
 impl DropRanges {
@@ -227,7 +227,7 @@ struct DropRangesBuilder {
     /// (see NodeInfo::drop_state). The hir_id_map field stores the mapping
     /// from HirIds to the HirIdIndex that is used to represent that value in
     /// bitvector.
-    tracked_value_map: FxHashMap<TrackedValue, TrackedValueIndex>,
+    tracked_value_map: UnordMap<TrackedValue, TrackedValueIndex>,
 
     /// When building the control flow graph, we don't always know the
     /// post-order index of the target node at the point we encounter it.

compiler/rustc_hir_typeck/src/generator_interior/drop_ranges/record_consumed_borrow.rs

@@ -4,7 +4,7 @@ use crate::{
     FnCtxt,
 };
 use hir::{def_id::DefId, Body, HirId, HirIdMap};
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::{fx::FxIndexSet, unord::UnordSet};
 use rustc_hir as hir;
 use rustc_middle::ty::{ParamEnv, TyCtxt};
 use rustc_middle::{
@@ -30,13 +30,13 @@ pub(super) struct ConsumedAndBorrowedPlaces {
     ///
     /// Note that this set excludes "partial drops" -- for example, a statement like `drop(x.y)` is
     /// not considered a drop of `x`, although it would be a drop of `x.y`.
-    pub(super) consumed: HirIdMap<FxHashSet<TrackedValue>>,
+    pub(super) consumed: HirIdMap<FxIndexSet<TrackedValue>>,
 
     /// A set of hir-ids of values or variables that are borrowed at some point within the body.
-    pub(super) borrowed: FxHashSet<TrackedValue>,
+    pub(super) borrowed: UnordSet<TrackedValue>,
 
     /// A set of hir-ids of values or variables that are borrowed at some point within the body.
-    pub(super) borrowed_temporaries: FxHashSet<HirId>,
+    pub(super) borrowed_temporaries: UnordSet<HirId>,
 }
 
 /// Works with ExprUseVisitor to find interesting values for the drop range analysis.

compiler/rustc_hir_typeck/src/inherited.rs

@@ -1,6 +1,6 @@
 use super::callee::DeferredCallResolution;
 
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::unord::{UnordMap, UnordSet};
 use rustc_hir as hir;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::HirIdMap;
@@ -61,9 +61,9 @@ pub struct Inherited<'tcx> {
     /// Whenever we introduce an adjustment from `!` into a type variable,
     /// we record that type variable here. This is later used to inform
     /// fallback. See the `fallback` module for details.
-    pub(super) diverging_type_vars: RefCell<FxHashSet<Ty<'tcx>>>,
+    pub(super) diverging_type_vars: RefCell<UnordSet<Ty<'tcx>>>,
 
-    pub(super) infer_var_info: RefCell<FxHashMap<ty::TyVid, ty::InferVarInfo>>,
+    pub(super) infer_var_info: RefCell<UnordMap<ty::TyVid, ty::InferVarInfo>>,
 }
 
 impl<'tcx> Deref for Inherited<'tcx> {

compiler/rustc_hir_typeck/src/lib.rs

@@ -6,7 +6,6 @@
 #![feature(min_specialization)]
 #![feature(control_flow_enum)]
 #![feature(option_as_slice)]
-#![allow(rustc::potential_query_instability)]
 #![recursion_limit = "256"]
 
 #[macro_use]