Skip to content

Commit fe36604

Browse files
committed
Lints unused assoc consts and unused trait with assoc tys
1 parent 0a1449a commit fe36604

8 files changed

+222
-60
lines changed

compiler/rustc_passes/src/dead.rs

Lines changed: 113 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use rustc_hir::{self as hir, Node, PatKind, TyKind};
1717
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
1818
use rustc_middle::middle::privacy::Level;
1919
use rustc_middle::query::Providers;
20-
use rustc_middle::ty::{self, AssocItemContainer, Ty, TyCtxt};
20+
use rustc_middle::ty::{self, AssocItemContainer, TyCtxt};
2121
use rustc_middle::{bug, span_bug};
2222
use rustc_session::lint;
2323
use rustc_session::lint::builtin::DEAD_CODE;
@@ -44,7 +44,9 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
4444
)
4545
}
4646

47-
fn adt_of<'tcx>(ty: &hir::Ty<'tcx>) -> Option<(LocalDefId, DefKind)> {
47+
/// Returns the local def id and def kind of the adt,
48+
/// if the given ty refers to one local adt definition
49+
fn adt_def_of_ty<'tcx>(ty: &hir::Ty<'tcx>) -> Option<(LocalDefId, DefKind)> {
4850
match ty.kind {
4951
TyKind::Path(hir::QPath::Resolved(_, path)) => {
5052
if let Res::Def(def_kind, def_id) = path.res
@@ -55,8 +57,8 @@ fn adt_of<'tcx>(ty: &hir::Ty<'tcx>) -> Option<(LocalDefId, DefKind)> {
5557
None
5658
}
5759
}
58-
TyKind::Slice(ty) | TyKind::Array(ty, _) => adt_of(ty),
59-
TyKind::Ptr(ty) | TyKind::Ref(_, ty) => adt_of(ty.ty),
60+
TyKind::Slice(ty) | TyKind::Array(ty, _) => adt_def_of_ty(ty),
61+
TyKind::Ptr(ty) | TyKind::Ref(_, ty) => adt_def_of_ty(ty.ty),
6062
_ => None,
6163
}
6264
}
@@ -114,7 +116,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
114116

115117
fn handle_res(&mut self, res: Res) {
116118
match res {
117-
Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::TyAlias, def_id) => {
119+
Res::Def(
120+
DefKind::AssocConst | DefKind::AssocTy | DefKind::Const | DefKind::TyAlias,
121+
def_id,
122+
) => {
118123
self.check_def_id(def_id);
119124
}
120125
_ if self.in_pat => {}
@@ -234,6 +239,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
234239
) {
235240
let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
236241
ty::Adt(adt, _) => {
242+
// mark the adt live if its variant appears as the pattern
243+
// considering cases when we have `let T(x) = foo()` and `fn foo<T>() -> T;`
244+
// we will lose the liveness info of `T` cause we cannot mark it live when visiting `foo`
245+
// related issue: https://github.com/rust-lang/rust/issues/120770
237246
self.check_def_id(adt.did());
238247
adt.variant_of_res(res)
239248
}
@@ -257,6 +266,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
257266
) {
258267
let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
259268
ty::Adt(adt, _) => {
269+
// mark the adt live if its variant appears as the pattern
270+
// considering cases when we have `let T(x) = foo()` and `fn foo<T>() -> T;`
271+
// we will lose the liveness info of `T` cause we cannot mark it live when visiting `foo`
272+
// related issue: https://github.com/rust-lang/rust/issues/120770
260273
self.check_def_id(adt.did());
261274
adt.variant_of_res(res)
262275
}
@@ -406,13 +419,21 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
406419
intravisit::walk_item(self, item)
407420
}
408421
hir::ItemKind::ForeignMod { .. } => {}
422+
hir::ItemKind::Trait(_, _, _, _, trait_item_refs) => {
423+
// mark assoc ty live if the trait is live
424+
for trait_item in trait_item_refs {
425+
if matches!(trait_item.kind, hir::AssocItemKind::Type) {
426+
self.check_def_id(trait_item.id.owner_id.to_def_id());
427+
}
428+
}
429+
intravisit::walk_item(self, item)
430+
}
409431
_ => intravisit::walk_item(self, item),
410432
},
411433
Node::TraitItem(trait_item) => {
412-
// mark corresponding ImplTerm live
434+
// mark the trait live
413435
let trait_item_id = trait_item.owner_id.to_def_id();
414436
if let Some(trait_id) = self.tcx.trait_of_item(trait_item_id) {
415-
// mark the trait live
416437
self.check_def_id(trait_id);
417438
}
418439

@@ -437,6 +458,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
437458
_ => {}
438459
}
439460
}
461+
440462
intravisit::walk_impl_item(self, impl_item);
441463
}
442464
Node::ForeignItem(foreign_item) => {
@@ -458,55 +480,44 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
458480
}
459481
}
460482

461-
fn solve_rest_items(&mut self, mut unsolved_items: Vec<(hir::ItemId, LocalDefId)>) {
462-
let mut ready;
463-
(ready, unsolved_items) =
464-
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
465-
self.item_should_be_checked(impl_id, local_def_id)
466-
});
467-
468-
while !ready.is_empty() {
469-
self.worklist =
470-
ready.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect();
471-
self.mark_live_symbols();
472-
473-
(ready, unsolved_items) =
474-
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
475-
self.item_should_be_checked(impl_id, local_def_id)
476-
});
477-
}
478-
}
479-
483+
/// Returns an impl or impl item should be checked or not
484+
/// `impl_id` is the `ItemId` of the impl or the impl item belongs to,
485+
/// `local_def_id` may point to the impl or the impl item,
486+
/// both impl and impl item that may pass to this function are of trait,
487+
/// and added into the unsolved_items during `create_and_seed_worklist`
480488
fn item_should_be_checked(&mut self, impl_id: hir::ItemId, local_def_id: LocalDefId) -> bool {
481489
let trait_def_id = match self.tcx.def_kind(local_def_id) {
482-
// for assoc impl items of traits, we concern the corresponding trait items are used or not
483-
DefKind::AssocFn => self
490+
// assoc impl items of traits are live if the corresponding trait items are live
491+
DefKind::AssocConst | DefKind::AssocTy | DefKind::AssocFn => self
484492
.tcx
485493
.associated_item(local_def_id)
486494
.trait_item_def_id
487495
.and_then(|def_id| def_id.as_local()),
488-
// for impl items, we concern the corresonding traits are used or not
496+
// impl items are live if the corresponding traits are live
489497
DefKind::Impl { of_trait: true } => self
490498
.tcx
491499
.impl_trait_ref(impl_id.owner_id.def_id)
492500
.and_then(|trait_ref| trait_ref.skip_binder().def_id.as_local()),
493501
_ => None,
494502
};
495503

496-
if !trait_def_id.map(|def_id| self.live_symbols.contains(&def_id)).unwrap_or(true) {
504+
if let Some(def_id) = trait_def_id
505+
&& !self.live_symbols.contains(&def_id)
506+
{
497507
return false;
498508
}
499509

500510
// we only check the ty is used or not for ADTs defined locally
501-
let ty_def_id = adt_of(self.tcx.hir().item(impl_id).expect_impl().self_ty).and_then(
502-
|(local_def_id, def_kind)| {
503-
matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
504-
.then_some(local_def_id)
505-
},
506-
);
507-
508511
// the impl/impl item is used if the trait/trait item is used and the ty is used
509-
ty_def_id.map(|def_id| self.live_symbols.contains(&def_id)).unwrap_or(true)
512+
if let Some((local_def_id, def_kind)) =
513+
adt_def_of_ty(self.tcx.hir().item(impl_id).expect_impl().self_ty)
514+
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
515+
&& !self.live_symbols.contains(&local_def_id)
516+
{
517+
return false;
518+
}
519+
520+
true
510521
}
511522
}
512523

@@ -632,6 +643,29 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
632643

633644
self.in_pat = in_pat;
634645
}
646+
647+
fn visit_poly_trait_ref(&mut self, t: &'tcx hir::PolyTraitRef<'tcx>) {
648+
// mark the assoc const appears in poly-trait-ref live
649+
if let Some(pathsegment) = t.trait_ref.path.segments.last()
650+
&& let Some(args) = pathsegment.args
651+
{
652+
for constraint in args.constraints {
653+
if let Some(item) = self
654+
.tcx
655+
.associated_items(pathsegment.res.def_id())
656+
.filter_by_name_unhygienic(constraint.ident.name)
657+
.find(|i| {
658+
matches!(i.kind, ty::AssocKind::Const)
659+
&& i.ident(self.tcx).normalize_to_macros_2_0() == constraint.ident
660+
})
661+
&& let Some(local_def_id) = item.def_id.as_local()
662+
{
663+
self.worklist.push((local_def_id, ComesFromAllowExpect::No));
664+
}
665+
}
666+
}
667+
intravisit::walk_poly_trait_ref(self, t);
668+
}
635669
}
636670

637671
fn has_allow_dead_code_or_lang_attr(
@@ -715,7 +749,7 @@ fn check_item<'tcx>(
715749
}
716750
DefKind::Impl { of_trait } => {
717751
// get DefIds from another query
718-
let local_def_ids = tcx
752+
let associated_item_def_ids = tcx
719753
.associated_item_def_ids(id.owner_id)
720754
.iter()
721755
.filter_map(|def_id| def_id.as_local());
@@ -729,14 +763,16 @@ fn check_item<'tcx>(
729763
}
730764

731765
// And we access the Map here to get HirId from LocalDefId
732-
for local_def_id in local_def_ids {
733-
if !matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) {
734-
worklist.push((local_def_id, ComesFromAllowExpect::No));
735-
} else if let Some(comes_from_allow) =
736-
has_allow_dead_code_or_lang_attr(tcx, local_def_id)
766+
for local_def_id in associated_item_def_ids {
767+
if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, local_def_id)
737768
{
738769
worklist.push((local_def_id, comes_from_allow));
739770
} else if of_trait {
771+
// we only care about assoc items of trait,
772+
// because they cannot be visited directly
773+
// so we mark them based on the trait/trait item
774+
// and self ty are checked first and both live,
775+
// but inherent assoc items can be visited and marked directly
740776
unsolved_items.push((id, local_def_id));
741777
}
742778
}
@@ -762,10 +798,13 @@ fn check_trait_item(
762798
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
763799
id: hir::TraitItemId,
764800
) {
765-
use hir::TraitItemKind::{Const, Fn};
766-
if matches!(tcx.def_kind(id.owner_id), DefKind::AssocConst | DefKind::AssocFn) {
801+
use hir::TraitItemKind::{Const, Fn, Type};
802+
if matches!(
803+
tcx.def_kind(id.owner_id),
804+
DefKind::AssocConst | DefKind::AssocTy | DefKind::AssocFn
805+
) {
767806
let trait_item = tcx.hir().trait_item(id);
768-
if matches!(trait_item.kind, Const(_, Some(_)) | Fn(..))
807+
if matches!(trait_item.kind, Const(_, Some(_)) | Type(..) | Fn(..))
769808
&& let Some(comes_from_allow) =
770809
has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id)
771810
{
@@ -807,7 +846,7 @@ fn create_and_seed_worklist(
807846
// checks impls and impl-items later
808847
match tcx.def_kind(id) {
809848
DefKind::Impl { of_trait } => !of_trait,
810-
DefKind::AssocFn => {
849+
DefKind::AssocConst | DefKind::AssocTy | DefKind::AssocFn => {
811850
// still check public trait items, and impl items not of trait
812851
let assoc_item = tcx.associated_item(id);
813852
!matches!(assoc_item.container, AssocItemContainer::ImplContainer)
@@ -844,7 +883,7 @@ fn live_symbols_and_ignored_derived_traits(
844883
tcx: TyCtxt<'_>,
845884
(): (),
846885
) -> (LocalDefIdSet, LocalDefIdMap<Vec<(DefId, DefId)>>) {
847-
let (worklist, struct_constructors, unsolved_items) = create_and_seed_worklist(tcx);
886+
let (worklist, struct_constructors, mut unsolved_items) = create_and_seed_worklist(tcx);
848887
let mut symbol_visitor = MarkSymbolVisitor {
849888
worklist,
850889
tcx,
@@ -857,8 +896,26 @@ fn live_symbols_and_ignored_derived_traits(
857896
struct_constructors,
858897
ignored_derived_traits: Default::default(),
859898
};
899+
860900
symbol_visitor.mark_live_symbols();
861-
symbol_visitor.solve_rest_items(unsolved_items);
901+
let mut rest_items_should_be_checked;
902+
(rest_items_should_be_checked, unsolved_items) =
903+
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
904+
symbol_visitor.item_should_be_checked(impl_id, local_def_id)
905+
});
906+
907+
while !rest_items_should_be_checked.is_empty() {
908+
symbol_visitor.worklist = rest_items_should_be_checked
909+
.into_iter()
910+
.map(|(_, id)| (id, ComesFromAllowExpect::No))
911+
.collect();
912+
symbol_visitor.mark_live_symbols();
913+
914+
(rest_items_should_be_checked, unsolved_items) =
915+
unsolved_items.into_iter().partition(|&(impl_id, local_def_id)| {
916+
symbol_visitor.item_should_be_checked(impl_id, local_def_id)
917+
});
918+
}
862919

863920
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
864921
}
@@ -1098,6 +1155,7 @@ impl<'tcx> DeadVisitor<'tcx> {
10981155
}
10991156
match self.tcx.def_kind(def_id) {
11001157
DefKind::AssocConst
1158+
| DefKind::AssocTy
11011159
| DefKind::AssocFn
11021160
| DefKind::Fn
11031161
| DefKind::Static { .. }
@@ -1134,7 +1192,11 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
11341192
let def_kind = tcx.def_kind(item.owner_id);
11351193

11361194
let mut dead_codes = Vec::new();
1137-
// if we have diagnosed the trait, do not diagnose unused assoc items
1195+
// only diagnose unused assoc items for inherient impl and used trait
1196+
// for unused assoc items in impls of trait,
1197+
// we have diagnosed them in the trait if they are unused,
1198+
// for unused assoc items in unused trait,
1199+
// we have diagnosed the unused trait
11381200
if matches!(def_kind, DefKind::Impl { of_trait: false })
11391201
|| (def_kind == DefKind::Trait && live_symbols.contains(&item.owner_id.def_id))
11401202
{
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
//@ check-pass
2+
3+
#![deny(dead_code)]
4+
5+
trait UInt: Copy + From<u8> {}
6+
7+
impl UInt for u16 {}
8+
9+
trait Int: Copy {
10+
type Unsigned: UInt;
11+
12+
fn as_unsigned(self) -> Self::Unsigned;
13+
}
14+
15+
impl Int for i16 {
16+
type Unsigned = u16;
17+
18+
fn as_unsigned(self) -> u16 {
19+
self as _
20+
}
21+
}
22+
23+
fn priv_func<T: Int>(x: u8, y: T) -> (T::Unsigned, T::Unsigned) {
24+
(T::Unsigned::from(x), y.as_unsigned())
25+
}
26+
27+
pub fn pub_func(x: u8, y: i16) -> (u16, u16) {
28+
priv_func(x, y)
29+
}
30+
31+
fn main() {}

tests/ui/lint/dead-code/unused-adt-impl-pub-trait-with-assoc-const.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
struct T1; //~ ERROR struct `T1` is never constructed
44
pub struct T2(i32); //~ ERROR field `0` is never read
5-
struct T3;
65

76
trait Trait1 { //~ ERROR trait `Trait1` is never used
87
const UNUSED: i32;
@@ -42,11 +41,4 @@ impl Trait2 for T2 {
4241
const USED: i32 = 0;
4342
}
4443

45-
impl Trait3 for T3 {
46-
const USED: i32 = 0;
47-
fn construct_self() -> Self {
48-
Self
49-
}
50-
}
51-
5244
fn main() {}

tests/ui/lint/dead-code/unused-adt-impl-pub-trait-with-assoc-const.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ LL | pub struct T2(i32);
2121
= help: consider removing this field
2222

2323
error: trait `Trait1` is never used
24-
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:7:7
24+
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:6:7
2525
|
2626
LL | trait Trait1 {
2727
| ^^^^^^
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#![deny(dead_code)]
2+
3+
trait Trait {
4+
const UNUSED_CONST: i32; //~ ERROR associated constant `UNUSED_CONST` is never used
5+
const USED_CONST: i32;
6+
7+
fn foo(&self) {}
8+
}
9+
10+
pub struct T(());
11+
12+
impl Trait for T {
13+
const UNUSED_CONST: i32 = 0;
14+
const USED_CONST: i32 = 1;
15+
}
16+
17+
fn main() {
18+
T(()).foo();
19+
T::USED_CONST;
20+
}

0 commit comments

Comments
 (0)