Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove PartialOrd, Ord from LocalDefId #90408

Merged
merged 3 commits into from
Dec 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions compiler/rustc_data_structures/src/graph/scc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
}
Expand All @@ -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()
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_data_structures/src/graph/vec_graph/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::cmp::Ord;

use crate::graph::{DirectedGraph, GraphSuccessors, WithNumEdges, WithNumNodes, WithSuccessors};
use rustc_index::vec::{Idx, IndexVec};

Expand All @@ -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();
Expand Down Expand Up @@ -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()
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_hir/src/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down
18 changes: 17 additions & 1 deletion compiler/rustc_hir/src/hir_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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<std::cmp::Ordering> {
Some(self.cmp(&other))
}
}

rustc_data_structures::define_id_collections!(HirIdMap, HirIdSet, HirId);
rustc_data_structures::define_id_collections!(ItemLocalMap, ItemLocalSet, ItemLocalId);

Expand Down
13 changes: 9 additions & 4 deletions compiler/rustc_index/src/bit_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_index/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
pierwill marked this conversation as resolved.
Show resolved Hide resolved
fn new(idx: usize) -> Self;

fn index(self) -> usize;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
pierwill marked this conversation as resolved.
Show resolved Hide resolved
for (def_id, encode_const, encode_opt) in keys_and_jobs.into_iter() {
debug_assert!(encode_const || encode_opt);

Expand Down
15 changes: 6 additions & 9 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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(..)
Expand All @@ -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),
)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/fast_reject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub type SimplifiedType = SimplifiedTypeGen<DefId>;
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, TyEncodable, TyDecodable)]
pub enum SimplifiedTypeGen<D>
where
D: Copy + Debug + Ord + Eq,
D: Copy + Debug + Eq,
{
BoolSimplifiedType,
CharSimplifiedType,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/def_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
32 changes: 31 additions & 1 deletion compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -434,6 +434,36 @@ pub struct SpanData {
pub parent: Option<LocalDefId>,
}

// Order spans by position in the file.
impl Ord for SpanData {
pierwill marked this conversation as resolved.
Show resolved Hide resolved
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: _,
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
} = 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<Ordering> {
Some(self.cmp(other))
}
}

impl SpanData {
#[inline]
pub fn span(&self) -> Span {
Expand Down