From 8df92485919ac660273ec8dfef61d7ebb0b67a86 Mon Sep 17 00:00:00 2001 From: pierwill Date: Sat, 30 Oct 2021 10:11:50 -0500 Subject: [PATCH 1/7] 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 5d9bc1b2e5168..826d0fe9ab40e 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 ed7afcc07b101..d813c887eee9a 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 64bd32b8ddc79..69572807e7c9d 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 39552eb9f3102..1482a96cae316 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 241cbe234874d..e2f2324f4a8fb 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 55ccfd0ad2380..e3c6528b21885 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 a6828e103dc5f..eeb907d01148b 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 fd8606e6929e2..1422537cd5060 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 c4043d9698cc1..04011552e31ed 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 8be95b2d95a20..e1f30fef44f99 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 64c2ef30b4d76..d15befbf28730 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 2934368dfeb89..3bbf2a0e45666 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 { From a4a8c241c7bcce3604a0ce130a65084101d0ab47 Mon Sep 17 00:00:00 2001 From: pierwill Date: Tue, 7 Dec 2021 11:03:53 -0600 Subject: [PATCH 2/7] Require Ord for rustc_index::SparseBitSet::last_set_in --- compiler/rustc_index/src/bit_set.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index e2f2324f4a8fb..5aa213cb70134 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -715,6 +715,10 @@ impl SparseBitSet { self.elems.iter() } + bit_relations_inherent_impls! {} +} + +impl SparseBitSet { fn last_set_in(&self, range: impl RangeBounds) -> Option { let mut last_leq = None; for e in self.iter() { @@ -724,8 +728,6 @@ impl SparseBitSet { } last_leq } - - bit_relations_inherent_impls! {} } /// A fixed-size bitset type with a hybrid representation: sparse when there @@ -802,7 +804,10 @@ impl HybridBitSet { /// 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) -> Option { + pub fn last_set_in(&self, range: impl RangeBounds) -> Option + where + T: Ord, + { match self { HybridBitSet::Sparse(sparse) => sparse.last_set_in(range), HybridBitSet::Dense(dense) => dense.last_set_in(range), From e6ff0bac1ec1271439ed6a7dd35f861e293cd025 Mon Sep 17 00:00:00 2001 From: pierwill Date: Tue, 7 Dec 2021 12:29:13 -0600 Subject: [PATCH 3/7] rustc `VecGraph`: require the index type to implement Ord --- compiler/rustc_data_structures/src/graph/scc/mod.rs | 7 ++++--- compiler/rustc_data_structures/src/graph/vec_graph/mod.rs | 8 +++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_data_structures/src/graph/scc/mod.rs b/compiler/rustc_data_structures/src/graph/scc/mod.rs index 508a084b311f5..7099ca7eb88c2 100644 --- a/compiler/rustc_data_structures/src/graph/scc/mod.rs +++ b/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 { all_successors: Vec, } -impl Sccs { +impl Sccs { pub fn new(graph: &(impl DirectedGraph + WithNumNodes + WithSuccessors)) -> Self { SccsConstruction::construct(graph) } @@ -85,7 +86,7 @@ impl DirectedGraph for Sccs { type Node = S; } -impl WithNumNodes for Sccs { +impl WithNumNodes for Sccs { fn num_nodes(&self) -> usize { self.num_sccs() } @@ -103,7 +104,7 @@ impl<'graph, N: Idx, S: Idx> GraphSuccessors<'graph> for Sccs { type Iter = std::iter::Cloned>; } -impl WithSuccessors for Sccs { +impl WithSuccessors for Sccs { fn successors(&self, node: S) -> >::Iter { self.successors(node).iter().cloned() } 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 826d0fe9ab40e..3d91bcade59a4 100644 --- a/compiler/rustc_data_structures/src/graph/vec_graph/mod.rs +++ b/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,10 +19,10 @@ pub struct VecGraph { edge_targets: Vec, } -impl 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_by_key(|&edge_pairs| (edge_pairs.0.index(), edge_pairs.1.index())); + edge_pairs.sort(); let num_edges = edge_pairs.len(); @@ -100,7 +102,7 @@ impl<'graph, N: Idx> GraphSuccessors<'graph> for VecGraph { type Iter = std::iter::Cloned>; } -impl WithSuccessors for VecGraph { +impl WithSuccessors for VecGraph { fn successors(&self, node: N) -> >::Iter { self.successors(node).iter().cloned() } From f3f865e6a15bc5a580572f9b836913b2a39737dd Mon Sep 17 00:00:00 2001 From: pierwill Date: Wed, 22 Dec 2021 12:53:59 -0600 Subject: [PATCH 4/7] Remove ordering traits from `HirId` Use indexmap where necessary to maintain ordering --- Cargo.lock | 2 ++ compiler/rustc_borrowck/Cargo.toml | 1 + .../rustc_borrowck/src/constraints/mod.rs | 4 ++-- .../src/region_infer/dump_mir.rs | 3 +-- .../rustc_borrowck/src/region_infer/mod.rs | 24 ++++++++----------- compiler/rustc_hir/src/hir_id.rs | 12 ---------- compiler/rustc_middle/src/mir/query.rs | 4 ++-- compiler/rustc_middle/src/ty/sty.rs | 4 ++-- compiler/rustc_typeck/Cargo.toml | 1 + compiler/rustc_typeck/src/check/upvar.rs | 6 ++--- 10 files changed, 24 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 961a3ba9f19cf..94ca40773c740 100644 --- a/Cargo.lock +++ b/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", diff --git a/compiler/rustc_borrowck/Cargo.toml b/compiler/rustc_borrowck/Cargo.toml index 75e9c69af4e5c..d8499e5a1d98a 100644 --- a/compiler/rustc_borrowck/Cargo.toml +++ b/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" diff --git a/compiler/rustc_borrowck/src/constraints/mod.rs b/compiler/rustc_borrowck/src/constraints/mod.rs index 98378a98684e2..927595335fae7 100644 --- a/compiler/rustc_borrowck/src/constraints/mod.rs +++ b/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>, } @@ -72,7 +72,7 @@ impl<'tcx> Index 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 diff --git a/compiler/rustc_borrowck/src/region_infer/dump_mir.rs b/compiler/rustc_borrowck/src/region_infer/dump_mir.rs index cfd3acb6bdebd..3e99fe0c2384d 100644 --- a/compiler/rustc_borrowck/src/region_infer/dump_mir.rs +++ b/compiler/rustc_borrowck/src/region_infer/dump_mir.rs @@ -71,8 +71,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - let mut constraints: Vec<_> = self.constraints.outlives().iter().collect(); - constraints.sort(); + let constraints: indexmap::IndexSet<_> = self.constraints.outlives().iter().collect(); for constraint in &constraints { let OutlivesConstraint { sup, sub, locations, category, variance_info: _ } = constraint; let (name, arg) = match locations { diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index b39a28f79aadd..2079f074cc1ce 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -611,8 +611,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: indexmap::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 +2005,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { .unwrap_or_else(|| ObligationCauseCode::MiscObligation); // Classify each of the constraints along the path. - let mut categorized_path: Vec> = path + let mut categorized_path: indexmap::IndexSet> = path .iter() .map(|constraint| { if constraint.category == ConstraintCategory::ClosureBounds { @@ -2130,7 +2129,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 +2150,19 @@ 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: indexmap::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) + categorized_path.shift_remove_index(0).unwrap() } crate fn universe_info(&self, universe: ty::UniverseIndex) -> UniverseInfo<'tcx> { @@ -2266,7 +2262,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, diff --git a/compiler/rustc_hir/src/hir_id.rs b/compiler/rustc_hir/src/hir_id.rs index 1482a96cae316..753193dbd413f 100644 --- a/compiler/rustc_hir/src/hir_id.rs +++ b/compiler/rustc_hir/src/hir_id.rs @@ -44,18 +44,6 @@ 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_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs index cb3f3850958ec..aa1feb437b52e 100644 --- a/compiler/rustc_middle/src/mir/query.rs +++ b/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, diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 8706661b25021..48f92669b7ed9 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -2276,7 +2276,7 @@ impl<'tcx> TyS<'tcx> { /// a miscompilation or unsoundness. /// /// When in doubt, use `VarianceDiagInfo::default()` -#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum VarianceDiagInfo<'tcx> { /// No additional information - this is the default. /// We will not add any additional information to error messages. @@ -2296,7 +2296,7 @@ pub enum VarianceDiagInfo<'tcx> { }, } -#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum VarianceDiagMutKind { /// A mutable raw pointer (`*mut T`) RawPtr, diff --git a/compiler/rustc_typeck/Cargo.toml b/compiler/rustc_typeck/Cargo.toml index 7e570e151c529..06b840440b488 100644 --- a/compiler/rustc_typeck/Cargo.toml +++ b/compiler/rustc_typeck/Cargo.toml @@ -27,3 +27,4 @@ rustc_infer = { path = "../rustc_infer" } rustc_trait_selection = { path = "../rustc_trait_selection" } rustc_ty_utils = { path = "../rustc_ty_utils" } rustc_lint = { path = "../rustc_lint" } +indexmap = "1.7.0" diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index ffd7d29bbbbee..e1f6d7c088000 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -33,6 +33,7 @@ use super::FnCtxt; use crate::expr_use_visitor as euv; +use indexmap::IndexSet; use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; use rustc_hir as hir; @@ -86,7 +87,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// Intermediate format to store the hir_id pointing to the use that resulted in the /// corresponding place being captured and a String which contains the captured value's /// name (i.e: a.b.c) -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] enum UpvarMigrationInfo { /// We previously captured all of `x`, but now we capture some sub-path. CapturingPrecise { source_expr: Option, var_name: String }, @@ -1186,8 +1187,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { capture_diagnostic.insert(key.clone()); } - let mut capture_diagnostic = capture_diagnostic.into_iter().collect::>(); - capture_diagnostic.sort(); + let capture_diagnostic: IndexSet<_> = capture_diagnostic.into_iter().collect(); for captures_info in capture_diagnostic { // Get the auto trait reasons of why migration is needed because of that capture, if there are any let capture_trait_reasons = From ab627aaef096883f4d47da3daa9a98905ebbcd24 Mon Sep 17 00:00:00 2001 From: pierwill Date: Wed, 22 Dec 2021 14:37:48 -0600 Subject: [PATCH 5/7] Fix off-by-one --- compiler/rustc_borrowck/src/region_infer/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 2079f074cc1ce..2d73acd5e4af4 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -2046,7 +2046,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. @@ -2162,7 +2162,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { return categorized_path[i].clone(); } - categorized_path.shift_remove_index(0).unwrap() + return categorized_path[0].clone(); } crate fn universe_info(&self, universe: ty::UniverseIndex) -> UniverseInfo<'tcx> { From 4ab2bdebb6f2e483d0a497825f8ebc36c72a3d2e Mon Sep 17 00:00:00 2001 From: pierwill Date: Wed, 22 Dec 2021 14:49:49 -0600 Subject: [PATCH 6/7] Bless 2 ui tests --- .../2229_closure_analysis/migrations/multi_diagnostics.stderr | 2 +- .../ui/closures/2229_closure_analysis/migrations/precise.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr index 0008f1b2c07ed..804d56a5afe1c 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/multi_diagnostics.stderr @@ -78,8 +78,8 @@ LL | let _f_1 = f1.1; LL | } | - | | - | in Rust 2018, `f1` is dropped here, but in Rust 2021, only `f1.0` will be dropped here as part of the closure | in Rust 2018, `f1` is dropped here, but in Rust 2021, only `f1.1` will be dropped here as part of the closure + | in Rust 2018, `f1` is dropped here, but in Rust 2021, only `f1.0` will be dropped here as part of the closure | = note: for more information, see help: add a dummy let to cause `f1` to be fully captured diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr index aa9b8672a0ffb..7d1391514c923 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/precise.stderr @@ -40,9 +40,9 @@ LL | let _x = u.1.0; LL | } | - | | - | in Rust 2018, `u` is dropped here, but in Rust 2021, only `u.0.0` will be dropped here as part of the closure | in Rust 2018, `u` is dropped here, but in Rust 2021, only `u.0.1` will be dropped here as part of the closure | in Rust 2018, `u` is dropped here, but in Rust 2021, only `u.1.0` will be dropped here as part of the closure + | in Rust 2018, `u` is dropped here, but in Rust 2021, only `u.0.0` will be dropped here as part of the closure | = note: for more information, see help: add a dummy let to cause `u` to be fully captured From 069c64c74699f5cb6cefa034dcec4d9166a27568 Mon Sep 17 00:00:00 2001 From: pierwill Date: Wed, 22 Dec 2021 14:56:40 -0600 Subject: [PATCH 7/7] Import indexmap --- compiler/rustc_borrowck/src/region_infer/dump_mir.rs | 3 ++- compiler/rustc_borrowck/src/region_infer/mod.rs | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/dump_mir.rs b/compiler/rustc_borrowck/src/region_infer/dump_mir.rs index 3e99fe0c2384d..8087ff1abfeea 100644 --- a/compiler/rustc_borrowck/src/region_infer/dump_mir.rs +++ b/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,7 +72,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - let constraints: indexmap::IndexSet<_> = self.constraints.outlives().iter().collect(); + 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 { diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 2d73acd5e4af4..911771c40e7d0 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/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,7 +613,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { #[instrument(skip(self, _body), level = "debug")] fn propagate_constraints(&mut self, _body: &Body<'tcx>) { debug!("constraints={:#?}", { - let constraints: indexmap::IndexSet<_> = self.constraints.outlives().iter().collect(); + 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))) @@ -2005,7 +2007,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { .unwrap_or_else(|| ObligationCauseCode::MiscObligation); // Classify each of the constraints along the path. - let mut categorized_path: indexmap::IndexSet> = path + let mut categorized_path: IndexSet> = path .iter() .map(|constraint| { if constraint.category == ConstraintCategory::ClosureBounds { @@ -2153,8 +2155,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { let mut blame_vec: Vec<_> = categorized_path.into_iter().collect(); blame_vec[i].category = ConstraintCategory::Return(ReturnConstraint::ClosureUpvar(field)); - let updated_categorized_path: indexmap::IndexSet<_> = - blame_vec.into_iter().collect(); + let updated_categorized_path: IndexSet<_> = blame_vec.into_iter().collect(); categorized_path = updated_categorized_path } }