Skip to content

Commit

Permalink
Rollup merge of #130764 - compiler-errors:inherent, r=estebank
Browse files Browse the repository at this point in the history
Separate collection of crate-local inherent impls from error tracking

#119895 changed the return type of the `crate_inherent_impls` query from `CrateInherentImpls` to `Result<CrateInherentImpls, ErrorGuaranteed>` to avoid needing to use the non-parallel-friendly `track_errors()` to track if an error was reporting from within the query... This was mostly fine until #121113, which stopped halting compilation when we hit an `Err(ErrorGuaranteed)` in the `crate_inherent_impls` query.

Thus we proceed onwards to typeck, and since a return type of `Result<CrateInherentImpls, ErrorGuaranteed>` means that the query can *either* return one of "the list inherent impls" or "error has been reported", later on when we want to assemble method or associated item candidates for inherent impls, we were just treating any `Err(ErrorGuaranteed)` return value as if Rust had no inherent impls defined anywhere at all! This leads to basically every inherent method call failing with an error, lol, which was reported in #127798.

This PR changes the `crate_inherent_impls` query to return `(CrateInherentImpls, Result<(), ErrorGuaranteed>)`, i.e. returning the inherent impls collected *and* whether an error was reported in the query itself. It firewalls the latter part of that query into a new `crate_inherent_impls_validity_check` just for the `ensure()` call.

This fixes #127798.
  • Loading branch information
tgross35 authored Sep 24, 2024
2 parents 08a8e68 + f896985 commit e691743
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 14 deletions.
1 change: 0 additions & 1 deletion clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ fn check_unsafe_derive_deserialize<'tcx>(
.tcx
.inherent_impls(def.did())
.into_iter()
.flatten()
.map(|imp_did| cx.tcx.hir().expect_item(imp_did.expect_local()))
.any(|imp| has_unsafe(cx, imp))
{
Expand Down
4 changes: 1 addition & 3 deletions clippy_lints/src/inherent_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleInherentImpl {
// List of spans to lint. (lint_span, first_span)
let mut lint_spans = Vec::new();

let Ok(impls) = cx.tcx.crate_inherent_impls(()) else {
return;
};
let (impls, _) = cx.tcx.crate_inherent_impls(());

for (&id, impl_ids) in &impls.inherent_impls {
if impl_ids.len() < 2
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/len_zero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,6 @@ fn check_for_is_empty(
.tcx
.inherent_impls(impl_ty)
.into_iter()
.flatten()
.flat_map(|&id| cx.tcx.associated_items(id).filter_by_name_unhygienic(is_empty))
.find(|item| item.kind == AssocKind::Fn);

Expand Down Expand Up @@ -616,7 +615,7 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
/// Checks the inherent impl's items for an `is_empty(self)` method.
fn has_is_empty_impl(cx: &LateContext<'_>, id: DefId) -> bool {
let is_empty = sym!(is_empty);
cx.tcx.inherent_impls(id).into_iter().flatten().any(|imp| {
cx.tcx.inherent_impls(id).into_iter().any(|imp| {
cx.tcx
.associated_items(*imp)
.filter_by_name_unhygienic(is_empty)
Expand Down
1 change: 0 additions & 1 deletion clippy_lints/src/methods/or_fun_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ pub(super) fn check<'tcx>(
cx.tcx
.inherent_impls(adt_def.did())
.into_iter()
.flatten()
.flat_map(|impl_id| cx.tcx.associated_items(impl_id).filter_by_name_unhygienic(sugg))
.find_map(|assoc| {
if assoc.fn_has_self_parameter
Expand Down
8 changes: 2 additions & 6 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -589,14 +589,11 @@ fn find_primitive_impls<'tcx>(tcx: TyCtxt<'tcx>, name: &str) -> impl Iterator<It
"f32" => SimplifiedType::Float(FloatTy::F32),
"f64" => SimplifiedType::Float(FloatTy::F64),
_ => {
return Result::<&[_], rustc_errors::ErrorGuaranteed>::Ok(&[])
.into_iter()
.flatten()
.copied();
return [].iter().copied();
},
};

tcx.incoherent_impls(ty).into_iter().flatten().copied()
tcx.incoherent_impls(ty).into_iter().copied()
}

fn non_local_item_children_by_name(tcx: TyCtxt<'_>, def_id: DefId, name: Symbol) -> Vec<Res> {
Expand Down Expand Up @@ -721,7 +718,6 @@ pub fn def_path_res(tcx: TyCtxt<'_>, path: &[&str]) -> Vec<Res> {
let inherent_impl_children = tcx
.inherent_impls(def_id)
.into_iter()
.flatten()
.flat_map(|&impl_def_id| item_children_by_name(tcx, impl_def_id, segment));

let direct_children = item_children_by_name(tcx, def_id, segment);
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1316,7 +1316,7 @@ pub fn deref_chain<'cx, 'tcx>(cx: &'cx LateContext<'tcx>, ty: Ty<'tcx>) -> impl
/// If you need this, you should wrap this call in `clippy_utils::ty::deref_chain().any(...)`.
pub fn get_adt_inherent_method<'a>(cx: &'a LateContext<'_>, ty: Ty<'_>, method_name: Symbol) -> Option<&'a AssocItem> {
if let Some(ty_did) = ty.ty_adt_def().map(AdtDef::did) {
cx.tcx.inherent_impls(ty_did).into_iter().flatten().find_map(|&did| {
cx.tcx.inherent_impls(ty_did).into_iter().find_map(|&did| {
cx.tcx
.associated_items(did)
.filter_by_name_unhygienic(method_name)
Expand Down

0 comments on commit e691743

Please sign in to comment.