[WIP] Remove ordering traits from HirId

Cargo.lock

@@ -3654,6 +3654,7 @@ name = "rustc_borrowck"
 version = "0.0.0"
 dependencies = [
  "either",
+ "indexmap",
  "itertools 0.9.0",
  "polonius-engine",
  "rustc_const_eval",
@@ -4568,6 +4569,7 @@ dependencies = [
 name = "rustc_typeck"
 version = "0.0.0"
 dependencies = [
+ "indexmap",
  "rustc_arena",
  "rustc_ast",
  "rustc_attr",

compiler/rustc_borrowck/Cargo.toml

@@ -28,3 +28,4 @@ rustc_target = { path = "../rustc_target" }
 rustc_trait_selection = { path = "../rustc_trait_selection" }
 rustc_traits = { path = "../rustc_traits" }
 rustc_span = { path = "../rustc_span" }
+indexmap = "1.7.0"

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -13,7 +13,7 @@ crate mod graph;
 /// constraints of the form `R1: R2`. Each constraint is identified by
 /// a unique `OutlivesConstraintIndex` and you can index into the set
 /// (`constraint_set[i]`) to access the constraint details.
-#[derive(Clone, Default)]
+#[derive(Clone, Default, Hash)]
 crate struct OutlivesConstraintSet<'tcx> {
     outlives: IndexVec<OutlivesConstraintIndex, OutlivesConstraint<'tcx>>,
 }
@@ -72,7 +72,7 @@ impl<'tcx> Index<OutlivesConstraintIndex> for OutlivesConstraintSet<'tcx> {
     }
 }
 
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Clone, PartialEq, Eq, Hash)]
 pub struct OutlivesConstraint<'tcx> {
     // NB. The ordering here is not significant for correctness, but
     // it is for convenience. Before we dump the constraints in the

compiler/rustc_borrowck/src/region_infer/dump_mir.rs

@@ -5,6 +5,7 @@
 
 use super::{OutlivesConstraint, RegionInferenceContext};
 use crate::type_check::Locations;
+use indexmap::IndexSet;
 use rustc_infer::infer::NllRegionVariableOrigin;
 use rustc_middle::ty::TyCtxt;
 use std::io::{self, Write};
@@ -71,8 +72,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             }
         }
 
-        let mut constraints: Vec<_> = self.constraints.outlives().iter().collect();
-        constraints.sort();
+        let constraints: IndexSet<_> = self.constraints.outlives().iter().collect();
         for constraint in &constraints {
             let OutlivesConstraint { sup, sub, locations, category, variance_info: _ } = constraint;
             let (name, arg) = match locations {

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -20,6 +20,8 @@ use rustc_middle::traits::ObligationCauseCode;
 use rustc_middle::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable};
 use rustc_span::Span;
 
+use indexmap::IndexSet;
+
 use crate::{
     constraints::{
         graph::NormalConstraintGraph, ConstraintSccIndex, OutlivesConstraint, OutlivesConstraintSet,
@@ -611,8 +613,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     #[instrument(skip(self, _body), level = "debug")]
     fn propagate_constraints(&mut self, _body: &Body<'tcx>) {
         debug!("constraints={:#?}", {
-            let mut constraints: Vec<_> = self.constraints.outlives().iter().collect();
-            constraints.sort();
+            let constraints: IndexSet<_> = self.constraints.outlives().iter().collect();
             constraints
                 .into_iter()
                 .map(|c| (c, self.constraint_sccs.scc(c.sup), self.constraint_sccs.scc(c.sub)))
@@ -2006,7 +2007,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             .unwrap_or_else(|| ObligationCauseCode::MiscObligation);
 
         // Classify each of the constraints along the path.
-        let mut categorized_path: Vec<BlameConstraint<'tcx>> = path
+        let mut categorized_path: IndexSet<BlameConstraint<'tcx>> = path
             .iter()
             .map(|constraint| {
                 if constraint.category == ConstraintCategory::ClosureBounds {
@@ -2047,7 +2048,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // we still want to screen for an "interesting" point to
         // highlight (e.g., a call site or something).
         let target_scc = self.constraint_sccs.scc(target_region);
-        let mut range = 0..path.len();
+        let mut range = 0..categorized_path.len();
 
         // As noted above, when reporting an error, there is typically a chain of constraints
         // leading from some "source" region which must outlive some "target" region.
@@ -2130,7 +2131,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         );
 
         if let Some(i) = best_choice {
-            if let Some(next) = categorized_path.get(i + 1) {
+            if let Some(next) = categorized_path.get_index(i + 1) {
                 if matches!(categorized_path[i].category, ConstraintCategory::Return(_))
                     && next.category == ConstraintCategory::OpaqueType
                 {
@@ -2151,22 +2152,18 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 });
 
                 if let Some(field) = field {
-                    categorized_path[i].category =
+                    let mut blame_vec: Vec<_> = categorized_path.into_iter().collect();
+                    blame_vec[i].category =
                         ConstraintCategory::Return(ReturnConstraint::ClosureUpvar(field));
+                    let updated_categorized_path: IndexSet<_> = blame_vec.into_iter().collect();
+                    categorized_path = updated_categorized_path
                 }
             }
 
             return categorized_path[i].clone();
         }
 
-        // If that search fails, that is.. unusual. Maybe everything
-        // is in the same SCC or something. In that case, find what
-        // appears to be the most interesting point to report to the
-        // user via an even more ad-hoc guess.
-        categorized_path.sort_by(|p0, p1| p0.category.cmp(&p1.category));
-        debug!("best_blame_constraint: sorted_path={:#?}", categorized_path);
-
-        categorized_path.remove(0)
+        return categorized_path[0].clone();
     }
 
     crate fn universe_info(&self, universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
@@ -2266,7 +2263,7 @@ impl<'tcx> ClosureRegionRequirementsExt<'tcx> for ClosureRegionRequirements<'tcx
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct BlameConstraint<'tcx> {
     pub category: ConstraintCategory,
     pub from_closure: bool,

compiler/rustc_data_structures/src/graph/scc/mod.rs

@@ -9,6 +9,7 @@ use crate::fx::FxHashSet;
 use crate::graph::vec_graph::VecGraph;
 use crate::graph::{DirectedGraph, GraphSuccessors, WithNumEdges, WithNumNodes, WithSuccessors};
 use rustc_index::vec::{Idx, IndexVec};
+use std::cmp::Ord;
 use std::ops::Range;
 
 #[cfg(test)]
@@ -38,7 +39,7 @@ struct SccData<S: Idx> {
     all_successors: Vec<S>,
 }
 
-impl<N: Idx, S: Idx> Sccs<N, S> {
+impl<N: Idx, S: Idx + Ord> Sccs<N, S> {
     pub fn new(graph: &(impl DirectedGraph<Node = N> + WithNumNodes + WithSuccessors)) -> Self {
         SccsConstruction::construct(graph)
     }
@@ -85,7 +86,7 @@ impl<N: Idx, S: Idx> DirectedGraph for Sccs<N, S> {
     type Node = S;
 }
 
-impl<N: Idx, S: Idx> WithNumNodes for Sccs<N, S> {
+impl<N: Idx, S: Idx + Ord> WithNumNodes for Sccs<N, S> {
     fn num_nodes(&self) -> usize {
         self.num_sccs()
     }
@@ -103,7 +104,7 @@ impl<'graph, N: Idx, S: Idx> GraphSuccessors<'graph> for Sccs<N, S> {
     type Iter = std::iter::Cloned<std::slice::Iter<'graph, S>>;
 }
 
-impl<N: Idx, S: Idx> WithSuccessors for Sccs<N, S> {
+impl<N: Idx, S: Idx + Ord> WithSuccessors for Sccs<N, S> {
     fn successors(&self, node: S) -> <Self as GraphSuccessors<'_>>::Iter {
         self.successors(node).iter().cloned()
     }

compiler/rustc_data_structures/src/graph/vec_graph/mod.rs

@@ -1,3 +1,5 @@
+use std::cmp::Ord;
+
 use crate::graph::{DirectedGraph, GraphSuccessors, WithNumEdges, WithNumNodes, WithSuccessors};
 use rustc_index::vec::{Idx, IndexVec};
 
@@ -17,7 +19,7 @@ pub struct VecGraph<N: Idx> {
     edge_targets: Vec<N>,
 }
 
-impl<N: Idx> VecGraph<N> {
+impl<N: Idx + Ord> VecGraph<N> {
     pub fn new(num_nodes: usize, mut edge_pairs: Vec<(N, N)>) -> Self {
         // Sort the edges by the source -- this is important.
         edge_pairs.sort();
@@ -100,7 +102,7 @@ impl<'graph, N: Idx> GraphSuccessors<'graph> for VecGraph<N> {
     type Iter = std::iter::Cloned<std::slice::Iter<'graph, N>>;
 }
 
-impl<N: Idx> WithSuccessors for VecGraph<N> {
+impl<N: Idx + Ord> WithSuccessors for VecGraph<N> {
     fn successors(&self, node: N) -> <Self as GraphSuccessors<'_>>::Iter {
         self.successors(node).iter().cloned()
     }

compiler/rustc_hir/src/definitions.rs

@@ -101,7 +101,11 @@ impl DefPathTable {
 pub struct Definitions {
     table: DefPathTable,
 
-    // FIXME(eddyb) ideally all `LocalDefId`s would be HIR owners.
+    /// Only [`LocalDefId`]s for items and item-like are HIR owners.
+    /// The associated `HirId` has a `local_id` of `0`.
+    /// Generic parameters and closures are also assigned a `LocalDefId` but are not HIR owners.
+    /// Their `HirId`s are defined by their position while lowering the enclosing owner.
+    // FIXME(cjgillot) Some `LocalDefId`s from `use` items are dropped during lowering and lack a `HirId`.
     pub(super) def_id_to_hir_id: IndexVec<LocalDefId, Option<hir::HirId>>,
     /// The reverse mapping of `def_id_to_hir_id`.
     pub(super) hir_id_to_def_id: FxHashMap<hir::HirId, LocalDefId>,

compiler/rustc_hir/src/hir.rs

@@ -1203,7 +1203,7 @@ pub enum UnsafeSource {
     UserProvided,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Hash, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Hash, Debug)]
 pub struct BodyId {
     pub hir_id: HirId,
 }
@@ -1980,7 +1980,7 @@ pub struct FnSig<'hir> {
 // The bodies for items are stored "out of line", in a separate
 // hashmap in the `Crate`. Here we just record the hir-id of the item
 // so it can fetched later.
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Debug)]
 pub struct TraitItemId {
     pub def_id: LocalDefId,
 }
@@ -2043,7 +2043,7 @@ pub enum TraitItemKind<'hir> {
 // The bodies for items are stored "out of line", in a separate
 // hashmap in the `Crate`. Here we just record the hir-id of the item
 // so it can fetched later.
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Debug)]
 pub struct ImplItemId {
     pub def_id: LocalDefId,
 }
@@ -2644,7 +2644,7 @@ impl<'hir> VariantData<'hir> {
 // The bodies for items are stored "out of line", in a separate
 // hashmap in the `Crate`. Here we just record the hir-id of the item
 // so it can fetched later.
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug, Hash)]
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Debug, Hash)]
 pub struct ItemId {
     pub def_id: LocalDefId,
 }
@@ -2883,7 +2883,7 @@ pub enum AssocItemKind {
 // The bodies for items are stored "out of line", in a separate
 // hashmap in the `Crate`. Here we just record the hir-id of the item
 // so it can fetched later.
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Debug)]
 pub struct ForeignItemId {
     pub def_id: LocalDefId,
 }

compiler/rustc_hir/src/hir_id.rs

@@ -11,7 +11,7 @@ use std::fmt;
 /// the `local_id` part of the `HirId` changing, which is a very useful property in
 /// incremental compilation where we have to persist things through changes to
 /// the code base.
-#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)]
+#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
 #[derive(Encodable, Decodable)]
 pub struct HirId {
     pub owner: LocalDefId,
@@ -32,6 +32,10 @@ impl HirId {
     pub fn make_owner(owner: LocalDefId) -> Self {
         Self { owner, local_id: ItemLocalId::from_u32(0) }
     }
+
+    pub fn index(self) -> (usize, usize) {
+        (rustc_index::vec::Idx::index(self.owner), rustc_index::vec::Idx::index(self.local_id))
+    }
 }
 
 impl fmt::Display for HirId {

compiler/rustc_index/src/bit_set.rs

@@ -675,7 +675,7 @@ impl<T: Idx> SparseBitSet<T> {
 
     fn insert(&mut self, elem: T) -> bool {
         assert!(elem.index() < self.domain_size);
-        let changed = if let Some(i) = self.elems.iter().position(|&e| e >= elem) {
+        let changed = if let Some(i) = self.elems.iter().position(|&e| e.index() >= elem.index()) {
             if self.elems[i] == elem {
                 // `elem` is already in the set.
                 false
@@ -715,6 +715,10 @@ impl<T: Idx> SparseBitSet<T> {
         self.elems.iter()
     }
 
+    bit_relations_inherent_impls! {}
+}
+
+impl<T: Idx + Ord> SparseBitSet<T> {
     fn last_set_in(&self, range: impl RangeBounds<T>) -> Option<T> {
         let mut last_leq = None;
         for e in self.iter() {
@@ -724,8 +728,6 @@ impl<T: Idx> SparseBitSet<T> {
         }
         last_leq
     }
-
-    bit_relations_inherent_impls! {}
 }
 
 /// A fixed-size bitset type with a hybrid representation: sparse when there
@@ -802,7 +804,10 @@ impl<T: Idx> HybridBitSet<T> {
     /// Returns the previous element present in the bitset from `elem`,
     /// inclusively of elem. That is, will return `Some(elem)` if elem is in the
     /// bitset.
-    pub fn last_set_in(&self, range: impl RangeBounds<T>) -> Option<T> {
+    pub fn last_set_in(&self, range: impl RangeBounds<T>) -> Option<T>
+    where
+        T: Ord,
+    {
         match self {
             HybridBitSet::Sparse(sparse) => sparse.last_set_in(range),
             HybridBitSet::Dense(dense) => dense.last_set_in(range),

compiler/rustc_index/src/vec.rs

@@ -12,7 +12,7 @@ use std::vec;
 /// Represents some newtyped `usize` wrapper.
 ///
 /// Purpose: avoid mixing indexes for different bitvector domains.
-pub trait Idx: Copy + 'static + Ord + Debug + Hash {
+pub trait Idx: Copy + 'static + Eq + PartialEq + Debug + Hash {
     fn new(idx: usize) -> Self;
 
     fn index(self) -> usize;

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1306,7 +1306,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             })
             .collect::<Vec<_>>();
         // Sort everything to ensure a stable order for diagnotics.
-        keys_and_jobs.sort_by_key(|&(def_id, _, _)| def_id);
+        keys_and_jobs.sort_by_key(|&(def_id, _, _)| def_id.index());
         for (def_id, encode_const, encode_opt) in keys_and_jobs.into_iter() {
             debug_assert!(encode_const || encode_opt);
 

compiler/rustc_middle/src/mir/mono.rs

@@ -6,7 +6,7 @@ use rustc_data_structures::fingerprint::Fingerprint;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
-use rustc_hir::{HirId, ItemId};
+use rustc_hir::ItemId;
 use rustc_query_system::ich::{NodeIdHashingMode, StableHashingContext};
 use rustc_session::config::OptLevel;
 use rustc_span::source_map::Span;
@@ -355,7 +355,7 @@ impl<'tcx> CodegenUnit<'tcx> {
         // The codegen tests rely on items being process in the same order as
         // they appear in the file, so for local items, we sort by node_id first
         #[derive(PartialEq, Eq, PartialOrd, Ord)]
-        pub struct ItemSortKey<'tcx>(Option<HirId>, SymbolName<'tcx>);
+        pub struct ItemSortKey<'tcx>(Option<usize>, SymbolName<'tcx>);
 
         fn item_sort_key<'tcx>(tcx: TyCtxt<'tcx>, item: MonoItem<'tcx>) -> ItemSortKey<'tcx> {
             ItemSortKey(
@@ -366,10 +366,7 @@ impl<'tcx> CodegenUnit<'tcx> {
                             // instances into account. The others don't matter for
                             // the codegen tests and can even make item order
                             // unstable.
-                            InstanceDef::Item(def) => def
-                                .did
-                                .as_local()
-                                .map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id)),
+                            InstanceDef::Item(def) => Some(def.did.index.as_usize()),
                             InstanceDef::VtableShim(..)
                             | InstanceDef::ReifyShim(..)
                             | InstanceDef::Intrinsic(..)
@@ -380,10 +377,10 @@ impl<'tcx> CodegenUnit<'tcx> {
                             | InstanceDef::CloneShim(..) => None,
                         }
                     }
-                    MonoItem::Static(def_id) => {
-                        def_id.as_local().map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id))
+                    MonoItem::Static(def_id) => Some(def_id.index.as_usize()),
+                    MonoItem::GlobalAsm(item_id) => {
+                        Some(item_id.def_id.to_def_id().index.as_usize())
                     }
-                    MonoItem::GlobalAsm(item_id) => Some(item_id.hir_id()),
                 },
                 item.symbol_name(tcx),
             )

compiler/rustc_middle/src/mir/query.rs

@@ -318,7 +318,7 @@ rustc_data_structures::static_assert_size!(ConstraintCategory, 12);
 /// order of the category, thereby influencing diagnostic output.
 ///
 /// See also `rustc_const_eval::borrow_check::constraints`.
-#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 #[derive(TyEncodable, TyDecodable, HashStable)]
 pub enum ConstraintCategory {
     Return(ReturnConstraint),
@@ -358,7 +358,7 @@ pub enum ConstraintCategory {
     Internal,
 }
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash)]
 #[derive(TyEncodable, TyDecodable, HashStable)]
 pub enum ReturnConstraint {
     Normal,