From 48fca125b8ff5c090b834fd52ba042b0a0c6a77b Mon Sep 17 00:00:00 2001 From: bors Date: Sun, 2 Jul 2023 21:12:10 +0000 Subject: [PATCH] Auto merge of #11069 - y21:issue11063, r=Alexendoo [`missing_fields_in_debug`]: make sure self type is an adt Fixes #11063, another ICE that can only happen in core. This lint needs the `DefId` of the implementor to get its fields, but that ICEs if the implementor does not have a `DefId` (as is the case with primitive types, e.g. `impl Debug for bool`), which is where this ICE comes from. This PR changes the check I added in #10897 to be more... robust against `Debug` implementations we don't want to lint. Instead of just checking if the self type is a type parameter and "special casing" one specific case we don't want to lint, we should probably rather just check that the self type is either a struct, an enum or a union and only then continue. That prevents weird edge cases like this one that can only happen in core. Again, I don't know if it's even possible to add a test case for this since one cannot implement `Debug` for primitive types outside of the crate that defined `Debug` (core). I did make sure that this PR no longer ICEs on `impl Debug for T` and `impl Debug for bool`. Maybe writing such a test is possible with `#![no_core]` and then re-defining the `Debug` trait or something like that...? changelog: [`missing_fields_in_debug`]: make sure self type is an adt (fixes an ICE in core) r? `@Alexendoo` (reviewed the last PRs for this lint) --- .../clippy_lints/src/missing_fields_in_debug.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/missing_fields_in_debug.rs b/src/tools/clippy/clippy_lints/src/missing_fields_in_debug.rs index 1138d1163a4a9..497514fbcd606 100644 --- a/src/tools/clippy/clippy_lints/src/missing_fields_in_debug.rs +++ b/src/tools/clippy/clippy_lints/src/missing_fields_in_debug.rs @@ -207,11 +207,10 @@ impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug { if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), self_ty, items, .. }) = item.kind && let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res && let TyKind::Path(QPath::Resolved(_, self_path)) = &self_ty.kind - // don't trigger if self is a generic parameter, e.g. `impl Debug for T` - // this can only happen in core itself, where the trait is defined, - // but it caused ICEs in the past: - // https://github.com/rust-lang/rust-clippy/issues/10887 - && !matches!(self_path.res, Res::Def(DefKind::TyParam, _)) + // make sure that the self type is either a struct, an enum or a union + // this prevents ICEs such as when self is a type parameter or a primitive type + // (see #10887, #11063) + && let Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Union, self_path_did) = self_path.res && cx.match_def_path(trait_def_id, &[sym::core, sym::fmt, sym::Debug]) // don't trigger if this impl was derived && !cx.tcx.has_attr(item.owner_id, sym::automatically_derived) @@ -222,7 +221,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug { && let body = cx.tcx.hir().body(*body_id) && let ExprKind::Block(block, _) = body.value.kind // inspect `self` - && let self_ty = cx.tcx.type_of(self_path.res.def_id()).skip_binder().peel_refs() + && let self_ty = cx.tcx.type_of(self_path_did).skip_binder().peel_refs() && let Some(self_adt) = self_ty.ty_adt_def() && let Some(self_def_id) = self_adt.did().as_local() && let Some(Node::Item(self_item)) = cx.tcx.hir().find_by_def_id(self_def_id)