user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Refactor the two-phase check for impls and impl items

Browse source
This commit is contained in:
Mu001999 2025-05-23 03:51:15 +08:00
parent 0fc6f1672b
commit f83ecd8270
6 changed files with 128 additions and 133 deletions
Download patch file Download diff file Expand all files Collapse all files

2
compiler/rustc_middle/src/query/mod.rs
View file

@ -1137,7 +1137,7 @@ rustc_queries! {
/// their respective impl (i.e., part of the derive macro)
query live_symbols_and_ignored_derived_traits(_: ()) -> &'tcx (
LocalDefIdSet,
LocalDefIdMap<Vec<(DefId, DefId)>>
LocalDefIdMap<FxIndexSet<(DefId, DefId)>>
) {
arena_cache
desc { "finding live symbols in crate" }

250
compiler/rustc_passes/src/dead.rs
View file

@ -8,12 +8,13 @@ use std::mem;
use hir::ItemKind;
use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
use rustc_abi::FieldIdx;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::MultiSpan;
use rustc_hir::def::{CtorOf, DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{self as hir, Node, PatKind, TyKind};
use rustc_hir::{self as hir, Node, PatKind, QPath, TyKind};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::privacy::Level;
use rustc_middle::query::Providers;
@ -44,15 +45,20 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
)
}
fn ty_ref_to_pub_struct(tcx: TyCtxt<'_>, ty: &hir::Ty<'_>) -> bool {
if let TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
&& let Res::Def(def_kind, def_id) = path.res
&& def_id.is_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
tcx.visibility(def_id).is_public()
} else {
true
/// Returns the local def id of the ADT if the given ty refers to a local one.
fn local_adt_def_of_ty<'tcx>(ty: &hir::Ty<'tcx>) -> Option<LocalDefId> {
match ty.kind {
TyKind::Path(QPath::Resolved(_, path)) => {
if let Res::Def(def_kind, def_id) = path.res
&& let Some(local_def_id) = def_id.as_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
Some(local_def_id)
} else {
None
}
}
_ => None,
}
}
@ -78,7 +84,7 @@ struct MarkSymbolVisitor<'tcx> {
// maps from ADTs to ignored derived traits (e.g. Debug and Clone)
// and the span of their respective impl (i.e., part of the derive
// macro)
ignored_derived_traits: LocalDefIdMap<Vec<(DefId, DefId)>>,
ignored_derived_traits: LocalDefIdMap<FxIndexSet<(DefId, DefId)>>,
}
impl<'tcx> MarkSymbolVisitor<'tcx> {
@ -360,7 +366,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
&& let Some(fn_sig) =
self.tcx.hir_fn_sig_by_hir_id(self.tcx.local_def_id_to_hir_id(local_def_id))
&& matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None)
&& let TyKind::Path(hir::QPath::Resolved(_, path)) =
&& let TyKind::Path(QPath::Resolved(_, path)) =
self.tcx.hir_expect_item(local_impl_of).expect_impl().self_ty.kind
&& let Res::Def(def_kind, did) = path.res
{
@ -388,7 +394,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
self.ignored_derived_traits
.entry(adt_def_id)
.or_default()
.push((trait_of, impl_of));
.insert((trait_of, impl_of));
}
return true;
}
@ -420,51 +426,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
intravisit::walk_item(self, item)
}
hir::ItemKind::ForeignMod { .. } => {}
hir::ItemKind::Trait(..) => {
for &impl_def_id in self.tcx.local_trait_impls(item.owner_id.def_id) {
if let ItemKind::Impl(impl_ref) = self.tcx.hir_expect_item(impl_def_id).kind
{
// skip items
// mark dependent traits live
intravisit::walk_generics(self, impl_ref.generics);
// mark dependent parameters live
intravisit::walk_path(self, impl_ref.of_trait.unwrap().path);
hir::ItemKind::Trait(.., trait_item_refs) => {
// mark assoc ty live if the trait is live
for trait_item in trait_item_refs {
if matches!(trait_item.kind, hir::AssocItemKind::Type) {
self.check_def_id(trait_item.id.owner_id.to_def_id());
}
}
intravisit::walk_item(self, item)
}
_ => intravisit::walk_item(self, item),
},
Node::TraitItem(trait_item) => {
// mark corresponding ImplTerm live
// mark the trait live
let trait_item_id = trait_item.owner_id.to_def_id();
if let Some(trait_id) = self.tcx.trait_of_item(trait_item_id) {
// mark the trait live
self.check_def_id(trait_id);
for impl_id in self.tcx.all_impls(trait_id) {
if let Some(local_impl_id) = impl_id.as_local()
&& let ItemKind::Impl(impl_ref) =
self.tcx.hir_expect_item(local_impl_id).kind
{
if !matches!(trait_item.kind, hir::TraitItemKind::Type(..))
&& !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty)
{
// skip methods of private ty,
// they would be solved in `solve_rest_impl_items`
continue;
}
// mark self_ty live
intravisit::walk_unambig_ty(self, impl_ref.self_ty);
if let Some(&impl_item_id) =
self.tcx.impl_item_implementor_ids(impl_id).get(&trait_item_id)
{
self.check_def_id(impl_item_id);
}
}
}
}
intravisit::walk_trait_item(self, trait_item);
}
@ -508,48 +485,58 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
}
}
fn solve_rest_impl_items(&mut self, mut unsolved_impl_items: Vec<(hir::ItemId, LocalDefId)>) {
let mut ready;
(ready, unsolved_impl_items) =
unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| {
self.impl_item_with_used_self(impl_id, impl_item_id)
});
while !ready.is_empty() {
self.worklist =
ready.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect();
self.mark_live_symbols();
(ready, unsolved_impl_items) =
unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| {
self.impl_item_with_used_self(impl_id, impl_item_id)
});
/// Returns whether `local_def_id` is potentially alive or not.
/// `local_def_id` points to an impl or an impl item,
/// both impl and impl item that may be passed to this function are of a trait,
/// and added into the unsolved_items during `create_and_seed_worklist`
fn check_impl_or_impl_item_live(
&mut self,
impl_id: hir::ItemId,
local_def_id: LocalDefId,
) -> bool {
if self.should_ignore_item(local_def_id.to_def_id()) {
return false;
}
}
fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId, impl_item_id: LocalDefId) -> bool {
if let TyKind::Path(hir::QPath::Resolved(_, path)) =
self.tcx.hir_item(impl_id).expect_impl().self_ty.kind
&& let Res::Def(def_kind, def_id) = path.res
&& let Some(local_def_id) = def_id.as_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
if self.tcx.visibility(impl_item_id).is_public() {
// for the public method, we don't know the trait item is used or not,
// so we mark the method live if the self is used
return self.live_symbols.contains(&local_def_id);
}
let trait_def_id = match self.tcx.def_kind(local_def_id) {
// assoc impl items of traits are live if the corresponding trait items are live
DefKind::AssocFn => self.tcx.associated_item(local_def_id).trait_item_def_id,
// impl items are live if the corresponding traits are live
DefKind::Impl { of_trait: true } => self
.tcx
.impl_trait_ref(impl_id.owner_id.def_id)
.and_then(|trait_ref| Some(trait_ref.skip_binder().def_id)),
_ => None,
};
if let Some(trait_item_id) = self.tcx.associated_item(impl_item_id).trait_item_def_id
&& let Some(local_id) = trait_item_id.as_local()
if let Some(trait_def_id) = trait_def_id {
if let Some(trait_def_id) = trait_def_id.as_local()
&& !self.live_symbols.contains(&trait_def_id)
{
// for the private method, we can know the trait item is used or not,
// so we mark the method live if the self is used and the trait item is used
return self.live_symbols.contains(&local_id)
&& self.live_symbols.contains(&local_def_id);
return false;
}
// FIXME: legacy logic to check whether the function may construct `Self`,
// this can be removed after supporting marking ADTs appearing in patterns
// as live, then we can check private impls of public traits directly
if let Some(fn_sig) =
self.tcx.hir_fn_sig_by_hir_id(self.tcx.local_def_id_to_hir_id(local_def_id))
&& matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None)
&& self.tcx.visibility(trait_def_id).is_public()
{
return true;
}
}
false
// The impl or impl item is used if the corresponding trait or trait item is used and the ty is used.
if let Some(local_def_id) =
local_adt_def_of_ty(self.tcx.hir_item(impl_id).expect_impl().self_ty)
&& !self.live_symbols.contains(&local_def_id)
{
return false;
}
true
}
}
@ -584,7 +571,7 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
match expr.kind {
hir::ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(..)) => {
hir::ExprKind::Path(ref qpath @ QPath::TypeRelative(..)) => {
let res = self.typeck_results().qpath_res(qpath, expr.hir_id);
self.handle_res(res);
}
@ -738,7 +725,7 @@ fn check_item<'tcx>(
tcx: TyCtxt<'tcx>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
struct_constructors: &mut LocalDefIdMap<LocalDefId>,
unsolved_impl_items: &mut Vec<(hir::ItemId, LocalDefId)>,
unsolved_items: &mut Vec<(hir::ItemId, LocalDefId)>,
id: hir::ItemId,
) {
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id);
@ -764,41 +751,33 @@ fn check_item<'tcx>(
}
}
DefKind::Impl { of_trait } => {
// get DefIds from another query
let local_def_ids = tcx
.associated_item_def_ids(id.owner_id)
.iter()
.filter_map(|def_id| def_id.as_local());
if let Some(comes_from_allow) =
has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id)
{
worklist.push((id.owner_id.def_id, comes_from_allow));
} else if of_trait {
unsolved_items.push((id, id.owner_id.def_id));
}
let ty_is_pub = ty_ref_to_pub_struct(tcx, tcx.hir_item(id).expect_impl().self_ty);
for def_id in tcx.associated_item_def_ids(id.owner_id) {
let local_def_id = def_id.expect_local();
// And we access the Map here to get HirId from LocalDefId
for local_def_id in local_def_ids {
// check the function may construct Self
let mut may_construct_self = false;
if let Some(fn_sig) =
tcx.hir_fn_sig_by_hir_id(tcx.local_def_id_to_hir_id(local_def_id))
{
may_construct_self =
matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None);
}
// for trait impl blocks,
// mark the method live if the self_ty is public,
// or the method is public and may construct self
if of_trait
&& (!matches!(tcx.def_kind(local_def_id), DefKind::AssocFn)
|| tcx.visibility(local_def_id).is_public()
&& (ty_is_pub || may_construct_self))
{
worklist.push((local_def_id, ComesFromAllowExpect::No));
} else if let Some(comes_from_allow) =
has_allow_dead_code_or_lang_attr(tcx, local_def_id)
if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, local_def_id)
{
worklist.push((local_def_id, comes_from_allow));
} else if of_trait {
// private method || public method not constructs self
unsolved_impl_items.push((id, local_def_id));
// FIXME: This condition can be removed
// if we support dead check for assoc consts and tys.
if !matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) {
worklist.push((local_def_id, ComesFromAllowExpect::No));
} else {
// We only care about associated items of traits,
// because they cannot be visited directly,
// so we later mark them as live if their corresponding traits
// or trait items and self types are both live,
// but inherent associated items can be visited and marked directly.
unsolved_items.push((id, local_def_id));
}
}
}
}
@ -892,8 +871,8 @@ fn create_and_seed_worklist(
fn live_symbols_and_ignored_derived_traits(
tcx: TyCtxt<'_>,
(): (),
) -> (LocalDefIdSet, LocalDefIdMap<Vec<(DefId, DefId)>>) {
let (worklist, struct_constructors, unsolved_impl_items) = create_and_seed_worklist(tcx);
) -> (LocalDefIdSet, LocalDefIdMap<FxIndexSet<(DefId, DefId)>>) {
let (worklist, struct_constructors, mut unsolved_items) = create_and_seed_worklist(tcx);
let mut symbol_visitor = MarkSymbolVisitor {
worklist,
tcx,
@ -907,7 +886,22 @@ fn live_symbols_and_ignored_derived_traits(
ignored_derived_traits: Default::default(),
};
symbol_visitor.mark_live_symbols();
symbol_visitor.solve_rest_impl_items(unsolved_impl_items);
let mut items_to_check;
(items_to_check, unsolved_items) =
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
symbol_visitor.check_impl_or_impl_item_live(impl_id, local_def_id)
});
while !items_to_check.is_empty() {
symbol_visitor.worklist =
items_to_check.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect();
symbol_visitor.mark_live_symbols();
(items_to_check, unsolved_items) =
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
symbol_visitor.check_impl_or_impl_item_live(impl_id, local_def_id)
});
}
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
}
@ -921,7 +915,7 @@ struct DeadItem {
struct DeadVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
live_symbols: &'tcx LocalDefIdSet,
ignored_derived_traits: &'tcx LocalDefIdMap<Vec<(DefId, DefId)>>,
ignored_derived_traits: &'tcx LocalDefIdMap<FxIndexSet<(DefId, DefId)>>,
}
enum ShouldWarnAboutField {
@ -1188,19 +1182,15 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
let def_kind = tcx.def_kind(item.owner_id);
let mut dead_codes = Vec::new();
// if we have diagnosed the trait, do not diagnose unused methods
if matches!(def_kind, DefKind::Impl { .. })
// Only diagnose unused assoc items in inherient impl and used trait,
// for unused assoc items in impls of trait,
// we have diagnosed them in the trait if they are unused,
// for unused assoc items in unused trait,
// we have diagnosed the unused trait.
if matches!(def_kind, DefKind::Impl { of_trait: false })
|| (def_kind == DefKind::Trait && live_symbols.contains(&item.owner_id.def_id))
{
for &def_id in tcx.associated_item_def_ids(item.owner_id.def_id) {
// We have diagnosed unused methods in traits
if matches!(def_kind, DefKind::Impl { of_trait: true })
&& tcx.def_kind(def_id) == DefKind::AssocFn
|| def_kind == DefKind::Trait && tcx.def_kind(def_id) != DefKind::AssocFn
{
continue;
}
if let Some(local_def_id) = def_id.as_local()
&& !visitor.is_live_code(local_def_id)
{

2
tests/ui/derives/clone-debug-dead-code.stderr
View file

@ -40,7 +40,7 @@ LL | struct D { f: () }
| |
| field in this struct
|
= note: `D` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis
= note: `D` has derived impls for the traits `Debug` and `Clone`, but these are intentionally ignored during dead code analysis
error: field `f` is never read
--> $DIR/clone-debug-dead-code.rs:21:12

3
tests/ui/deriving/deriving-in-macro.rs
View file

@ -1,5 +1,6 @@
//@ run-pass
//@ check-pass
#![allow(non_camel_case_types)]
#![allow(dead_code)]
macro_rules! define_vec {
() => (

2
tests/ui/lint/dead-code/issue-41883.stderr
View file

@ -29,6 +29,8 @@ error: struct `UnusedStruct` is never constructed
|
LL | struct UnusedStruct;
| ^^^^^^^^^^^^
|
= note: `UnusedStruct` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis
error: aborting due to 4 previous errors

2
tests/ui/lint/dead-code/multiple-dead-codes-in-the-same-struct.stderr
View file

@ -56,6 +56,8 @@ warning: struct `Foo` is never constructed
|
LL | struct Foo(usize, #[allow(unused)] usize);
| ^^^
|
= note: `Foo` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis
error: aborting due to 2 previous errors; 2 warnings emitted