8000 Remove `PartialOrd`, `Ord` from `DefId` by pierwill · Pull Request #90749 · rust-lang/rust · GitHub
[go: up one dir, main page]

Skip to content

Remove PartialOrd, Ord from DefId #90749

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
rest of huge changes from before
  • Loading branch information
pierwill committed Jan 18, 2022
commit f0fce21ba9b09b8dd7578ff3a8c044fd6f9f5597
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/region_infer/dump_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
}
}

let mut constraints: Vec<_> = self.constraints.outlives().iter().collect();
constraints.sort();
let constraints: Vec<_> = self.constraints.outlives().iter().collect();
// constraints.sort();
for constraint in &constraints {
let OutlivesConstraint { sup, sub, locations, category, variance_info: _ } = constraint;
let (name, arg) = match locations {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,8 @@ 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: Vec<_> = self.constraints.outlives().iter().collect();
// constraints.sort();
constraints
.into_iter()
.map(|c| (c, self.constraint_sccs.scc(c.sup), self.constraint_sccs.scc(c.sub)))
Expand Down
61 changes: 59 additions & 2 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,64 @@ fn add_unused_functions<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
continue;
}

debug!("generating unused fn: {:?}", non_codegenned_def_id);
cx.define_unused_fn(non_codegenned_def_id);
if unused_def_ids_by_file.is_empty() {
// There are no unused functions with file names to add (in any CGU)
return;
}

// Each `CodegenUnit` (CGU) has its own function_coverage_map, and generates a specific binary
// with its own coverage map.
//
// Each covered function `Instance` can be included in only one coverage map, produced from a
// specific function_coverage_map, from a specific CGU.
//
// Since unused functions did not generate code, they are not associated with any CGU yet.
//
// To avoid injecting the unused functions in multiple coverage maps (for multiple CGUs)
// determine which function_coverage_map has the responsibility for publishing unreachable
// coverage, based on file name: For each unused function, find the CGU that generates the
// first function (based on sorted `DefId`) from the same file.
//
// Add a new `FunctionCoverage` to the `function_coverage_map`, with unreachable code regions
// for each region in it's MIR.

let mut first_covered_def_id_by_file: FxHashMap<Symbol, DefId> = FxHashMap::default();
for &def_id in codegenned_def_ids.iter() {
if let Some(covered_file_name) = tcx.covered_file_name(def_id) {
// Only add files known to have unused functions
if unused_def_ids_by_file.contains_key(covered_file_name) {
first_covered_def_id_by_file.entry(*covered_file_name).or_insert(def_id);
}
}
}

// Get the set of def_ids with coverage regions, known by *this* CoverageContext.
let cgu_covered_def_ids: DefIdSet = match cx.coverage_context() {
Some(ctx) => ctx
.function_coverage_map
.borrow()
.keys()
.map(|&instance| instance.def.def_id())
.collect(),
None => return,
};

let cgu_covered_files: FxHashSet<Symbol> =
first_covered_def_id_by_file
.iter()
.filter_map(|(&file_name, def_id)| {
if cgu_covered_def_ids.contains(def_id) { Some(file_name) } else { None }
})
.collect();

// For each file for which this CGU is responsible for adding unused function coverage,
// get the `def_id`s for each unused function (if any), define a synthetic function with a
// single LLVM coverage counter, and add the function's coverage `CodeRegion`s. to the
// function_coverage_map.
for covered_file_name in cgu_covered_files {
for def_id in unused_def_ids_by_file.remove(&covered_file_name).into_iter().flatten() {
cx.define_unused_fn(def_id);
}
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
let all_len = self.place_cache.len();
self.place_cache.sort_unstable();
self.place_cache.sort_by_key(|b| b.local);
self.place_cache.dedup();
let has_duplicates = all_len != self.place_cache.len();
if has_duplicates {
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use rustc_span::symbol::Symbol;
use rustc_span::Span;

use std::cell::{Cell, Ref, RefCell};
use std::collections::BTreeMap;
use std::fmt;

use self::combine::CombineFields;
Expand Down Expand Up @@ -1500,7 +1499,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
span: Span,
lbrct: LateBoundRegionConversionTime,
value: ty::Binder<'tcx, T>,
) -> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
) -> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
where
T: TypeFoldable<'tcx>,
{
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_infer/src/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use rustc_middle::ty::{ReLateBound, ReVar};
use rustc_middle::ty::{Region, RegionVid};
use rustc_span::Span;

use std::collections::BTreeMap;
use std::ops::Range;
use std::{cmp, fmt, mem};

Expand Down Expand Up @@ -90,7 +89,7 @@ pub type VarInfos = IndexVec<RegionVid, RegionVariableInfo>;
pub struct RegionConstraintData<'tcx> {
/// Constraints of the form `A <= B`, where either `A` or `B` can
/// be a region variable (or neither, as it happens).
pub constraints: BTreeMap<Constraint<'tcx>, SubregionOrigin<'tcx>>,
pub constraints: FxHashMap<Constraint<'tcx>, SubregionOrigin<'tcx>>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does any code depend on the iteration order?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes:


/// Constraints of the form `R0 member of [R1, ..., Rn]`, meaning that
/// `R0` must be equal to one of the regions `R1..Rn`. These occur
Expand Down Expand Up @@ -127,7 +126,7 @@ pub struct RegionConstraintData<'tcx> {
}

/// Represents a constraint that influences the inference process.
#[derive(Clone, Copy, PartialEq, Eq, Debug, PartialOrd, Ord)]
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
pub enum Constraint<'tcx> {
/// A region variable is a subregion of another.
VarSubVar(RegionVid, RegionVid),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ rustc_index = { path = "../rustc_index" }
rustc_serialize = { path = "../rustc_serialize" }
rustc_ast = { path = "../rustc_ast" }
rustc_span = { path = "../rustc_span" }
chalk-ir = "0.75.0"
chalk-ir = "0.76.0"
smallvec = { version = "1.6.1", features = ["union", "may_dangle"] }
rustc_session = { path = "../rustc_session" }
rustc_type_ir = { path = "../rustc_type_ir" }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#![feature(get_mut_unchecked)]
#![feature(if_let_guard)]
#![feature(map_first_last)]
#![feature(negative_impls)]
#![feature(never_type)]
#![feature(extern_types)]
#![feature(new_uninit)]
Expand Down
2 changes: 1 addition & 1 deletion 558 compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1841,7 +1841,7 @@ rustc_index::newtype_index! {
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct PlaceRef<'tcx> {
pub local: Local,
pub projection: &'tcx [PlaceElem<'tcx>],
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ rustc_queries! {
}

/// Return all `impl` blocks in the current crate.
query all_local_trait_impls(_: ()) -> &'tcx BTreeMap<DefId, Vec<LocalDefId>> {
query all_local_trait_impls(_: ()) -> &'tcx FxHashMap<DefId, Vec<LocalDefId>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible iteration order matters here?

Copy link
Member Author
@pierwill pierwill Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the iteration order matters, we may need to use an FxIndexMap and audit how these maps are built.

desc { "local trait impls" }
}

Expand Down
15 changes: 0 additions & 15 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use rustc_span::symbol::sym;
use rustc_target::abi::VariantIdx;

use std::cell::RefCell;
use std::cmp::Ordering;
use std::hash::{Hash, Hasher};
use std::ops::Range;
use std::str;
Expand Down Expand Up @@ -100,20 +99,6 @@ pub struct AdtDef {
pub repr: ReprOptions,
}

impl PartialOrd for AdtDef {
fn partial_cmp(&self, other: &AdtDef) -> Option<Ordering> {
Some(self.cmp(&other))
}
}

/// There should be only one AdtDef for each `did`, therefore
/// it is fine to implement `Ord` only based on `did`.
impl Ord for AdtDef {
fn cmp(&self, other: &AdtDef) -> Ordering {
self.did.cmp(&other.did)
}
}

/// There should be only one AdtDef for each `did`, therefore
/// it is fine to implement `PartialEq` only based on `did`.
impl PartialEq for AdtDef {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub use kind::*;
pub use valtree::*;

/// Typed constant value.
#[derive(Copy, Clone, Debug, Hash, TyEncodable, TyDecodable, Eq, PartialEq, Ord, PartialOrd)]
#[derive(Copy, Clone, Debug, Hash, TyEncodable, TyDecodable, Eq, PartialEq)]
#[derive(HashStable)]
pub struct Const<'tcx> {
pub ty: Ty<'tcx>,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/consts/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_target::abi::Size;

use super::ScalarInt;
/// An unevaluated, potentially generic, constant.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, TyEncodable, TyDecodable, Lift)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, TyEncodable, TyDecodable, Lift)]
#[derive(Hash, HashStable)]
pub struct Unevaluated<'tcx, P = Option<Promoted>> {
pub def: ty::WithOptConstParam<DefId>,
Expand Down Expand Up @@ -43,7 +43,7 @@ impl<'tcx, P: Default> Unevaluated<'tcx, P> {
}

/// Represents a constant in Rust.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, TyEncodable, TyDecodable)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, TyEncodable, TyDecodable)]
#[derive(Hash, HashStable)]
pub enum ConstKind<'tcx> {
/// A const generic parameter.
Expand Down
11 changes: 5 additions & 6 deletions compiler/rustc_middle/src/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ use crate::ty::{self, flags::FlagComputation, Binder, Ty, TyCtxt, TypeFlags};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::sso::SsoHashSet;
use std::collections::BTreeMap;
use std::fmt;
use std::ops::ControlFlow;

Expand Down Expand Up @@ -695,12 +694,12 @@ impl<'tcx> TyCtxt<'tcx> {
self,
value: Binder<'tcx, T>,
mut fld_r: F,
) -> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
) -> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this FxHashMap used anyhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not? 👀 🤔 I'm not entirely sure how to check. The lsp-find-references command in Emacs shows several places where the returned map is ignored:

compiler/rustc_borrowck/src/universal_regions.rs
736:         let (value, _map) = self.tcx.replace_late_bound_regions(value, |br| {
compiler/rustc_middle/src/ty/fold.rs
821:         self.replace_late_bound_regions(value, |br| {
911:         self.replace_late_bound_regions(value, |_| self.lifetimes.re_erased).0
928:             .replace_late_bound_regions(sig, |_| {
compiler/rustc_middle/src/ty/print/pretty.rs
2160:             self.tcx.replace_late_bound_regions(value.clone(), |br| {
compiler/rustc_typeck/src/collect.rs
457:                                             .replace_late_bound_regions(poly_trait_ref, |_| {

where
F: FnMut(ty::BoundRegion) -> ty::Region<'tcx>,
T: TypeFoldable<'tcx>,
{
let mut region_map = BTreeMap::new();
let mut region_map = FxHashMap::default();
let mut real_fld_r =
|br: ty::BoundRegion| *region_map.entry(br).or_insert_with(|| fld_r(br));
let value = value.skip_binder();
Expand Down Expand Up @@ -747,14 +746,14 @@ impl<'tcx> TyCtxt<'tcx> {
mut fld_r: F,
fld_t: G,
fld_c: H,
) -> (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)
) -> (T, FxHashMap<ty::BoundRegion, ty::Region<'tcx>>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does any use site rely on the iteration order?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe here?

for &late_bound_region in map.values() {

where
F: FnMut(ty::BoundRegion) -> ty::Region<'tcx>,
G: FnMut(ty::BoundTy) -> Ty<'tcx>,
H: FnMut(ty::BoundVar, Ty<'tcx>) -> &'tcx ty::Const<'tcx>,
T: TypeFoldable<'tcx>,
{
let mut region_map = BTreeMap::new();
let mut region_map = FxHashMap::default();
let real_fld_r = |br: ty::BoundRegion| *region_map.entry(br).or_insert_with(|| fld_r(br));
let value = self.replace_escaping_bound_vars(value.skip_binder(), real_fld_r, fld_t, fld_c);
(value, region_map)
Expand Down
20 changes: 3 additions & 17 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_span::{sym, Span};
use rustc_target::abi::Align;

use std::cmp::Ordering;
use std::collections::BTreeMap;
use std::hash::{Hash, Hasher};
use std::ops::ControlFlow;
use std::{fmt, ptr, str};
Expand Down Expand Up @@ -134,7 +132,7 @@ pub struct ResolverOutputs {
/// via `extern crate` item and not `--extern` option or compiler built-in.
pub extern_prelude: FxHashMap<Symbol, bool>,
pub main_def: Option<MainDefinition>,
pub trait_impls: BTreeMap<DefId, Vec<LocalDefId>>,
pub trait_impls: FxHashMap<DefId, Vec<LocalDefId>>,
/// A list of proc macro LocalDefIds, written out in the order in which
/// they are declared in the static array generated by proc_macro_harness.
pub proc_macros: Vec<LocalDefId>,
Expand Down Expand Up @@ -421,18 +419,6 @@ impl<'tcx> TyS<'tcx> {
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
static_assert_size!(TyS<'_>, 40);

impl<'tcx> Ord for TyS<'tcx> {
fn cmp(&self, other: &TyS<'tcx>) -> Ordering {
self.kind().cmp(other.kind())
}
}

impl<'tcx> PartialOrd for TyS<'tcx> {
fn partial_cmp(&self, other: &TyS<'tcx>) -> Option<Ordering> {
Some(self.kind().cmp(other.kind()))
}
}

impl<'tcx> PartialEq for TyS<'tcx> {
#[inline]
fn eq(&self, other: &TyS<'tcx>) -> bool {
Expand Down Expand Up @@ -1101,7 +1087,7 @@ pub type PlaceholderRegion = Placeholder<BoundRegionKind>;
pub type PlaceholderType = Placeholder<BoundVar>;

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
#[derive(TyEncodable, TyDecodable, PartialOrd, Ord)]
#[derive(TyEncodable, TyDecodable)]
pub struct BoundConst<'tcx> {
pub var: BoundVar,
pub ty: Ty<'tcx>,
Expand Down Expand Up @@ -1161,7 +1147,7 @@ pub type PlaceholderConst<'tcx> = Placeholder<BoundConst<'tcx>>;
/// Meaning that we need to use `type_of(const_param_did)` if `const_param_did` is `Some`
/// to get the type of `did`.
#[derive(Copy, Clone, Debug, TypeFoldable, Lift, TyEncodable, TyDecodable)]
#[derive(PartialEq, Eq, PartialOrd, Ord)]
#[derive(PartialEq, Eq)]
#[derive(Hash, HashStable)]
pub struct WithOptConstParam<T> {
pub did: T,
Expand Down
Loading
0