From 8df92485919ac660273ec8dfef61d7ebb0b67a86 Mon Sep 17 00:00:00 2001 From: pierwill Date: Sat, 30 Oct 2021 10:11:50 -0500 Subject: [PATCH] Remove `PartialOrd` and `Ord` from `LocalDefId` Implement `Ord`, `PartialOrd` for SpanData --- .../src/graph/vec_graph/mod.rs | 2 +- compiler/rustc_hir/src/definitions.rs | 6 +++- compiler/rustc_hir/src/hir.rs | 10 +++--- compiler/rustc_hir/src/hir_id.rs | 18 ++++++++++- compiler/rustc_index/src/bit_set.rs | 2 +- compiler/rustc_index/src/vec.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_middle/src/mir/mono.rs | 15 ++++----- compiler/rustc_middle/src/ty/fast_reject.rs | 2 +- compiler/rustc_mir_transform/src/inline.rs | 3 +- compiler/rustc_span/src/def_id.rs | 2 +- compiler/rustc_span/src/lib.rs | 32 ++++++++++++++++++- 12 files changed, 72 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_data_structures/src/graph/vec_graph/mod.rs b/compiler/rustc_data_structures/src/graph/vec_graph/mod.rs index 5d9bc1b2e51..826d0fe9ab4 100644 --- a/compiler/rustc_data_structures/src/graph/vec_graph/mod.rs +++ b/compiler/rustc_data_structures/src/graph/vec_graph/mod.rs @@ -20,7 +20,7 @@ pub struct VecGraph { impl VecGraph { 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(); + edge_pairs.sort_by_key(|&edge_pairs| (edge_pairs.0.index(), edge_pairs.1.index())); let num_edges = edge_pairs.len(); diff --git a/compiler/rustc_hir/src/definitions.rs b/compiler/rustc_hir/src/definitions.rs index ed7afcc07b1..d813c887eee 100644 --- a/compiler/rustc_hir/src/definitions.rs +++ b/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>, /// The reverse mapping of `def_id_to_hir_id`. pub(super) hir_id_to_def_id: FxHashMap, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 64bd32b8ddc..69572807e7c 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/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, } diff --git a/compiler/rustc_hir/src/hir_id.rs b/compiler/rustc_hir/src/hir_id.rs index 39552eb9f31..1482a96cae3 100644 --- a/compiler/rustc_hir/src/hir_id.rs +++ b/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 { @@ -40,6 +44,18 @@ impl fmt::Display for HirId { } } +impl Ord for HirId { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + (self.index()).cmp(&(other.index())) + } +} + +impl PartialOrd for HirId { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(&other)) + } +} + rustc_data_structures::define_id_collections!(HirIdMap, HirIdSet, HirId); rustc_data_structures::define_id_collections!(ItemLocalMap, ItemLocalSet, ItemLocalId); diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 241cbe23487..e2f2324f4a8 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -675,7 +675,7 @@ impl SparseBitSet { 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 diff --git a/compiler/rustc_index/src/vec.rs b/compiler/rustc_index/src/vec.rs index 55ccfd0ad23..e3c6528b218 100644 --- a/compiler/rustc_index/src/vec.rs +++ b/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; diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index a6828e103dc..eeb907d0114 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1306,7 +1306,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { }) .collect::>(); // 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); diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index fd8606e6929..1422537cd50 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/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, SymbolName<'tcx>); + pub struct ItemSortKey<'tcx>(Option, 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), ) diff --git a/compiler/rustc_middle/src/ty/fast_reject.rs b/compiler/rustc_middle/src/ty/fast_reject.rs index c4043d9698c..04011552e31 100644 --- a/compiler/rustc_middle/src/ty/fast_reject.rs +++ b/compiler/rustc_middle/src/ty/fast_reject.rs @@ -20,7 +20,7 @@ pub type SimplifiedType = SimplifiedTypeGen; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, TyEncodable, TyDecodable)] pub enum SimplifiedTypeGen where - D: Copy + Debug + Ord + Eq, + D: Copy + Debug + Eq, { BoolSimplifiedType, CharSimplifiedType, diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 8be95b2d95a..e1f30fef44f 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -212,7 +212,8 @@ impl<'tcx> Inliner<'tcx> { // a lower `HirId` than the callee. This ensures that the callee will // not inline us. This trick only works without incremental compilation. // So don't do it if that is enabled. - if !self.tcx.dep_graph.is_fully_enabled() && self.hir_id < callee_hir_id { + if !self.tcx.dep_graph.is_fully_enabled() && self.hir_id.index() < callee_hir_id.index() + { return Ok(()); } diff --git a/compiler/rustc_span/src/def_id.rs b/compiler/rustc_span/src/def_id.rs index 64c2ef30b4d..d15befbf287 100644 --- a/compiler/rustc_span/src/def_id.rs +++ b/compiler/rustc_span/src/def_id.rs @@ -322,7 +322,7 @@ rustc_data_structures::define_id_collections!(DefIdMap, DefIdSet, DefId); /// few cases where we know that only DefIds from the local crate are expected /// and a DefId from a different crate would signify a bug somewhere. This /// is when LocalDefId comes in handy. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Hash)] pub struct LocalDefId { pub local_def_index: DefIndex, } diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 2934368dfeb..3bbf2a0e456 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -424,7 +424,7 @@ impl FileName { /// `SpanData` is public because `Span` uses a thread-local interner and can't be /// sent to other threads, but some pieces of performance infra run in a separate thread. /// Using `Span` is generally preferred. -#[derive(Clone, Copy, Hash, PartialEq, Eq, Ord, PartialOrd)] +#[derive(Clone, Copy, Hash, PartialEq, Eq)] pub struct SpanData { pub lo: BytePos, pub hi: BytePos, @@ -434,6 +434,36 @@ pub struct SpanData { pub parent: Option, } +// Order spans by position in the file. +impl Ord for SpanData { + fn cmp(&self, other: &Self) -> Ordering { + let SpanData { + lo: s_lo, + hi: s_hi, + ctxt: s_ctxt, + // `LocalDefId` does not implement `Ord`. + // The other fields are enough to determine in-file order. + parent: _, + } = self; + let SpanData { + lo: o_lo, + hi: o_hi, + ctxt: o_ctxt, + // `LocalDefId` does not implement `Ord`. + // The other fields are enough to determine in-file order. + parent: _, + } = other; + + (s_lo, s_hi, s_ctxt).cmp(&(o_lo, o_hi, o_ctxt)) + } +} + +impl PartialOrd for SpanData { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl SpanData { #[inline] pub fn span(&self) -> Span {