Skip to content

Remove unnecessary impl sorting in queries and metadata #120812

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

Merged
merged 4 commits into from
Jul 22, 2024
Merged
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
3 changes: 1 addition & 2 deletions compiler/rustc_infer/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
@@ -177,13 +177,12 @@ pub fn report_object_safety_error<'tcx>(
)));
}
impls => {
let mut types = impls
let types = impls
.iter()
.map(|t| {
with_no_trimmed_paths!(format!(" {}", tcx.type_of(*t).instantiate_identity(),))
})
.collect::<Vec<_>>();
types.sort();
err.help(format!(
"the following types implement the trait, consider defining an enum where each \
variant holds one of these types, implementing `{}` for this new enum and using \
5 changes: 3 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ use crate::rmeta::*;
use rustc_ast as ast;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::owned_slice::OwnedSlice;
use rustc_data_structures::sync::{Lock, Lrc, OnceLock};
use rustc_data_structures::unhash::UnhashMap;
@@ -83,12 +84,12 @@ pub(crate) struct CrateMetadata {
/// Trait impl data.
/// FIXME: Used only from queries and can use query cache,
/// so pre-decoding can probably be avoided.
trait_impls: FxHashMap<(u32, DefIndex), LazyArray<(DefIndex, Option<SimplifiedType>)>>,
trait_impls: FxIndexMap<(u32, DefIndex), LazyArray<(DefIndex, Option<SimplifiedType>)>>,
/// Inherent impls which do not follow the normal coherence rules.
///
/// These can be introduced using either `#![rustc_coherence_is_core]`
/// or `#[rustc_allow_incoherent_impl]`.
incoherent_impls: FxHashMap<SimplifiedType, LazyArray<DefIndex>>,
incoherent_impls: FxIndexMap<SimplifiedType, LazyArray<DefIndex>>,
/// Proc macro descriptions for this crate, if it's a proc macro crate.
raw_proc_macros: Option<&'static [ProcMacro]>,
/// Source maps for code from the crate.
83 changes: 18 additions & 65 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ use crate::errors::{FailCreateFileEncoder, FailWriteFile};
use crate::rmeta::*;

use rustc_ast::Attribute;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_data_structures::memmap::{Mmap, MmapMut};
use rustc_data_structures::sync::{join, par_for_each_in, Lrc};
use rustc_data_structures::temp_dir::MaybeTempDir;
@@ -13,7 +13,6 @@ use rustc_hir_pretty::id_to_string;
use rustc_middle::middle::dependency_format::Linkage;
use rustc_middle::middle::exported_symbols::metadata_symbol_name;
use rustc_middle::mir::interpret;
use rustc_middle::query::LocalCrate;
use rustc_middle::query::Providers;
use rustc_middle::traits::specialization_graph;
use rustc_middle::ty::codec::TyEncoder;
@@ -1508,10 +1507,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}

let inherent_impls = tcx.with_stable_hashing_context(|hcx| {
tcx.crate_inherent_impls(()).unwrap().inherent_impls.to_sorted(&hcx, true)
});
for (def_id, impls) in inherent_impls {
for (def_id, impls) in &tcx.crate_inherent_impls(()).unwrap().inherent_impls {
record_defaulted_array!(self.tables.inherent_impls[def_id.to_def_id()] <- impls.iter().map(|def_id| {
assert!(def_id.is_local());
def_id.index
@@ -1992,8 +1988,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
fn encode_impls(&mut self) -> LazyArray<TraitImpls> {
empty_proc_macro!(self);
let tcx = self.tcx;
let mut fx_hash_map: FxHashMap<DefId, Vec<(DefIndex, Option<SimplifiedType>)>> =
FxHashMap::default();
let mut trait_impls: FxIndexMap<DefId, Vec<(DefIndex, Option<SimplifiedType>)>> =
FxIndexMap::default();

for id in tcx.hir().items() {
let DefKind::Impl { of_trait } = tcx.def_kind(id.owner_id) else {
@@ -2012,7 +2008,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
trait_ref.self_ty(),
TreatParams::AsCandidateKey,
);
fx_hash_map
trait_impls
.entry(trait_ref.def_id)
.or_default()
.push((id.owner_id.def_id.local_def_index, simplified_self_ty));
@@ -2033,47 +2029,30 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}

let mut all_impls: Vec<_> = fx_hash_map.into_iter().collect();

// Bring everything into deterministic order for hashing
all_impls.sort_by_cached_key(|&(trait_def_id, _)| tcx.def_path_hash(trait_def_id));

let all_impls: Vec<_> = all_impls
let trait_impls: Vec<_> = trait_impls
.into_iter()
.map(|(trait_def_id, mut impls)| {
// Bring everything into deterministic order for hashing
impls.sort_by_cached_key(|&(index, _)| {
tcx.hir().def_path_hash(LocalDefId { local_def_index: index })
});

TraitImpls {
trait_id: (trait_def_id.krate.as_u32(), trait_def_id.index),
impls: self.lazy_array(&impls),
}
.map(|(trait_def_id, impls)| TraitImpls {
trait_id: (trait_def_id.krate.as_u32(), trait_def_id.index),
impls: self.lazy_array(&impls),
})
.collect();

self.lazy_array(&all_impls)
self.lazy_array(&trait_impls)
}

#[instrument(level = "debug", skip(self))]
fn encode_incoherent_impls(&mut self) -> LazyArray<IncoherentImpls> {
empty_proc_macro!(self);
let tcx = self.tcx;
let all_impls = tcx.with_stable_hashing_context(|hcx| {
tcx.crate_inherent_impls(()).unwrap().incoherent_impls.to_sorted(&hcx, true)
});

let all_impls: Vec<_> = all_impls
.into_iter()
.map(|(&simp, impls)| {
let mut impls: Vec<_> =
impls.into_iter().map(|def_id| def_id.local_def_index).collect();
impls.sort_by_cached_key(|&local_def_index| {
tcx.hir().def_path_hash(LocalDefId { local_def_index })
});

IncoherentImpls { self_ty: simp, impls: self.lazy_array(impls) }
let all_impls: Vec<_> = tcx
.crate_inherent_impls(())
.unwrap()
.incoherent_impls
.iter()
.map(|(&simp, impls)| IncoherentImpls {
self_ty: simp,
impls: self.lazy_array(impls.iter().map(|def_id| def_id.local_def_index)),
})
.collect();

@@ -2317,32 +2296,6 @@ pub fn provide(providers: &mut Providers) {
span_bug!(tcx.def_span(def_id), "no traits in scope for a doc link")
})
},
traits: |tcx, LocalCrate| {
let mut traits = Vec::new();
for id in tcx.hir().items() {
if matches!(tcx.def_kind(id.owner_id), DefKind::Trait | DefKind::TraitAlias) {
traits.push(id.owner_id.to_def_id())
}
}

// Bring everything into deterministic order.
traits.sort_by_cached_key(|&def_id| tcx.def_path_hash(def_id));
tcx.arena.alloc_slice(&traits)
},
trait_impls_in_crate: |tcx, LocalCrate| {
let mut trait_impls = Vec::new();
for id in tcx.hir().items() {
if matches!(tcx.def_kind(id.owner_id), DefKind::Impl { .. })
&& tcx.impl_trait_ref(id.owner_id).is_some()
{
trait_impls.push(id.owner_id.to_def_id())
}
}

// Bring everything into deterministic order.
trait_impls.sort_by_cached_key(|&def_id| tcx.def_path_hash(def_id));
tcx.arena.alloc_slice(&trait_impls)
},

..*providers
}
7 changes: 4 additions & 3 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -40,7 +40,6 @@ use rustc_data_structures::intern::Interned;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::steal::Steal;
use rustc_data_structures::tagged_ptr::CopyTaggedPtr;
use rustc_data_structures::unord::UnordMap;
use rustc_errors::{Diag, ErrorGuaranteed, StashKey};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, CtorOf, DefKind, DocLinkResMap, LifetimeRes, Res};
@@ -2083,6 +2082,8 @@ pub fn provide(providers: &mut Providers) {
*providers = Providers {
trait_impls_of: trait_def::trait_impls_of_provider,
incoherent_impls: trait_def::incoherent_impls_provider,
trait_impls_in_crate: trait_def::trait_impls_in_crate_provider,
traits: trait_def::traits_provider,
const_param_default: consts::const_param_default,
vtable_allocation: vtable::vtable_allocation_provider,
..*providers
@@ -2096,8 +2097,8 @@ pub fn provide(providers: &mut Providers) {
/// (constructing this map requires touching the entire crate).
#[derive(Clone, Debug, Default, HashStable)]
pub struct CrateInherentImpls {
pub inherent_impls: LocalDefIdMap<Vec<DefId>>,
pub incoherent_impls: UnordMap<SimplifiedType, Vec<LocalDefId>>,
pub inherent_impls: FxIndexMap<LocalDefId, Vec<DefId>>,
pub incoherent_impls: FxIndexMap<SimplifiedType, Vec<LocalDefId>>,
}

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, TyEncodable, HashStable)]
39 changes: 33 additions & 6 deletions compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
use crate::traits::specialization_graph;
use crate::ty::fast_reject::{self, SimplifiedType, TreatParams};
use crate::ty::{Ident, Ty, TyCtxt};
use hir::def_id::LOCAL_CRATE;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use std::iter;
use tracing::debug;

use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_macros::{Decodable, Encodable, HashStable};

use crate::query::LocalCrate;
use crate::traits::specialization_graph;
use crate::ty::fast_reject::{self, SimplifiedType, TreatParams};
use crate::ty::{Ident, Ty, TyCtxt};

/// A trait's definition with type information.
#[derive(HashStable, Encodable, Decodable)]
pub struct TraitDef {
@@ -274,3 +277,27 @@ pub(super) fn incoherent_impls_provider(

Ok(tcx.arena.alloc_slice(&impls))
}

pub(super) fn traits_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> &[DefId] {
let mut traits = Vec::new();
for id in tcx.hir().items() {
if matches!(tcx.def_kind(id.owner_id), DefKind::Trait | DefKind::TraitAlias) {
traits.push(id.owner_id.to_def_id())
}
}

tcx.arena.alloc_slice(&traits)
}

pub(super) fn trait_impls_in_crate_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> &[DefId] {
let mut trait_impls = Vec::new();
for id in tcx.hir().items() {
if matches!(tcx.def_kind(id.owner_id), DefKind::Impl { .. })
&& tcx.impl_trait_ref(id.owner_id).is_some()
{
trait_impls.push(id.owner_id.to_def_id())
}
}

tcx.arena.alloc_slice(&trait_impls)
}
15 changes: 7 additions & 8 deletions src/tools/clippy/clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
@@ -56,19 +56,18 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
let Ok(impls) = cx.tcx.crate_inherent_impls(()) else {
return;
};
let inherent_impls = cx
.tcx
.with_stable_hashing_context(|hcx| impls.inherent_impls.to_sorted(&hcx, true));

for (_, impl_ids) in inherent_impls.into_iter().filter(|(&id, impls)| {
impls.len() > 1
for (&id, impl_ids) in &impls.inherent_impls {
if impl_ids.len() < 2
// Check for `#[allow]` on the type definition
&& !is_lint_allowed(
|| is_lint_allowed(
cx,
MULTIPLE_INHERENT_IMPL,
cx.tcx.local_def_id_to_hir_id(id),
)
}) {
) {
continue;
}

for impl_id in impl_ids.iter().map(|id| id.expect_local()) {
let impl_ty = cx.tcx.type_of(impl_id).instantiate_identity();
match type_map.entry(impl_ty) {
4 changes: 2 additions & 2 deletions tests/rustdoc/anchor-id-duplicate-method-name-25001.rs
Original file line number Diff line number Diff line change
@@ -24,14 +24,14 @@ impl Foo<u32> {
}

impl<T> Bar for Foo<T> {
//@ has - '//*[@id="associatedtype.Item-1"]//h4[@class="code-header"]' 'type Item = T'
// @has - '//*[@id="associatedtype.Item"]//h4[@class="code-header"]' 'type Item = T'
type Item=T;

//@ has - '//*[@id="method.quux"]//h4[@class="code-header"]' 'fn quux(self)'
fn quux(self) {}
}
impl<'a, T> Bar for &'a Foo<T> {
//@ has - '//*[@id="associatedtype.Item"]//h4[@class="code-header"]' "type Item = &'a T"
// @has - '//*[@id="associatedtype.Item-1"]//h4[@class="code-header"]' "type Item = &'a T"
type Item=&'a T;

//@ has - '//*[@id="method.quux-1"]//h4[@class="code-header"]' 'fn quux(self)'
Original file line number Diff line number Diff line change
@@ -13,8 +13,8 @@ LL | type A<'a> where Self: 'a;
| ^ ...because it contains the generic associated type `A`
= help: consider moving `A` to another trait
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `Foo` for this new enum and using it instead:
Fooer<T>
Fooy
Fooer<T>

error[E0038]: the trait `Foo` cannot be made into an object
--> $DIR/gat-in-trait-path.rs:32:5
@@ -31,8 +31,8 @@ LL | type A<'a> where Self: 'a;
| ^ ...because it contains the generic associated type `A`
= help: consider moving `A` to another trait
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `Foo` for this new enum and using it instead:
Fooer<T>
Fooy
Fooer<T>

error[E0038]: the trait `Foo` cannot be made into an object
--> $DIR/gat-in-trait-path.rs:32:5
@@ -49,8 +49,8 @@ LL | type A<'a> where Self: 'a;
| ^ ...because it contains the generic associated type `A`
= help: consider moving `A` to another trait
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `Foo` for this new enum and using it instead:
Fooer<T>
Fooy
Fooer<T>
= note: required for the cast from `Box<Fooer<{integer}>>` to `Box<(dyn Foo<A = &'a ()> + 'static)>`

error: aborting due to 3 previous errors
4 changes: 2 additions & 2 deletions tests/ui/generic-associated-types/issue-79422.base.stderr
Original file line number Diff line number Diff line change
@@ -29,8 +29,8 @@ LL | type VRefCont<'a>: RefCont<'a, V> where Self: 'a;
| ^^^^^^^^ ...because it contains the generic associated type `VRefCont`
= help: consider moving `VRefCont` to another trait
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `MapLike` for this new enum and using it instead:
Source
std::collections::BTreeMap<K, V>
Source

error[E0038]: the trait `MapLike` cannot be made into an object
--> $DIR/issue-79422.rs:44:13
@@ -47,8 +47,8 @@ LL | type VRefCont<'a>: RefCont<'a, V> where Self: 'a;
| ^^^^^^^^ ...because it contains the generic associated type `VRefCont`
= help: consider moving `VRefCont` to another trait
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `MapLike` for this new enum and using it instead:
Source
std::collections::BTreeMap<K, V>
Source
= note: required for the cast from `Box<BTreeMap<u8, u8>>` to `Box<dyn MapLike<u8, u8, VRefCont = (dyn RefCont<'_, u8> + 'static)>>`

error: aborting due to 3 previous errors
4 changes: 2 additions & 2 deletions tests/ui/wf/wf-unsafe-trait-obj-match.stderr
Original file line number Diff line number Diff line change
@@ -26,8 +26,8 @@ LL | trait Trait: Sized {}
| |
| this trait cannot be made into an object...
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `Trait` for this new enum and using it instead:
R
S
R
= note: required for the cast from `&S` to `&dyn Trait`

error[E0038]: the trait `Trait` cannot be made into an object
@@ -48,8 +48,8 @@ LL | trait Trait: Sized {}
| |
| this trait cannot be made into an object...
= help: the following types implement the trait, consider defining an enum where each variant holds one of these types, implementing `Trait` for this new enum and using it instead:
R
S
R
= note: required for the cast from `&R` to `&dyn Trait`

error: aborting due to 3 previous errors