Skip to content

Commit

Permalink
Lint unused assoc tys although the trait is used
Browse files Browse the repository at this point in the history
  • Loading branch information
mu001999 committed Jul 16, 2024
1 parent 03c2100 commit a3531fb
Show file tree
Hide file tree
Showing 16 changed files with 164 additions and 22 deletions.
113 changes: 105 additions & 8 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_hir::{Node, PatKind, TyKind};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::privacy::Level;
use rustc_middle::query::Providers;
use rustc_middle::ty::{self, AssocItemContainer, TyCtxt};
use rustc_middle::ty::{self, AssocItemContainer, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor};
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_session::lint::builtin::DEAD_CODE;
Expand Down Expand Up @@ -450,7 +450,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
intravisit::walk_item(self, item)
}
hir::ItemKind::ForeignMod { .. } => {}
hir::ItemKind::Trait(_, _, _, _, trait_item_refs) => {
hir::ItemKind::Trait(..) => {
for impl_def_id in self.tcx.all_impls(item.owner_id.to_def_id()) {
if let Some(local_def_id) = impl_def_id.as_local()
&& let ItemKind::Impl(impl_ref) =
Expand All @@ -463,12 +463,13 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
intravisit::walk_path(self, impl_ref.of_trait.unwrap().path);
}
}
// mark assoc ty live if the trait is live
for trait_item in trait_item_refs {
if let hir::AssocItemKind::Type = trait_item.kind {
self.check_def_id(trait_item.id.owner_id.to_def_id());
}
}
intravisit::walk_item(self, item)
}
hir::ItemKind::Fn(..) => {
// check `T::Ty` in the types of inputs and output
// the result of type_of maybe different from the fn sig,
// so we also check the fn sig
self.visit_middle_fn_sig(item.owner_id.def_id);
intravisit::walk_item(self, item)
}
_ => intravisit::walk_item(self, item),
Expand Down Expand Up @@ -504,6 +505,21 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
}
}
}

match trait_item.kind {
hir::TraitItemKind::Fn(..) => {
// check `T::Ty` in the types of inputs and output
// the result of type_of maybe different from the fn sig,
// so we also check the fn sig
self.visit_middle_fn_sig(trait_item.owner_id.def_id)
}
hir::TraitItemKind::Type(.., Some(_)) | hir::TraitItemKind::Const(..) => {
// check `type X = T::Ty;` or `const X: T::Ty;`
self.visit_middle_ty_by_def_id(trait_item.owner_id.def_id)
}
_ => (),
}

intravisit::walk_trait_item(self, trait_item);
}
Node::ImplItem(impl_item) => {
Expand All @@ -525,6 +541,20 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
_ => {}
}
}

match impl_item.kind {
hir::ImplItemKind::Fn(..) => {
// check `T::Ty` in the types of inputs and output
// the result of type_of maybe different from the fn sig,
// so we also check the fn sig
self.visit_middle_fn_sig(impl_item.owner_id.def_id)
}
hir::ImplItemKind::Type(..) | hir::ImplItemKind::Const(..) => {
// check `type X = T::Ty;` or `const X: T::Ty;`
self.visit_middle_ty_by_def_id(impl_item.owner_id.def_id)
}
}

intravisit::walk_impl_item(self, impl_item);
}
Node::ForeignItem(foreign_item) => {
Expand Down Expand Up @@ -587,6 +617,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
false
}
}

fn visit_middle_ty(&mut self, ty: Ty<'tcx>) {
<Self as TypeVisitor<TyCtxt<'tcx>>>::visit_ty(self, ty);
}

fn visit_middle_ty_by_def_id(&mut self, def_id: LocalDefId) {
self.visit_middle_ty(self.tcx.type_of(def_id).instantiate_identity());
}

fn visit_middle_fn_sig(&mut self, def_id: LocalDefId) {
let fn_sig = self.tcx.fn_sig(def_id).instantiate_identity();
for ty in fn_sig.inputs().skip_binder() {
self.visit_middle_ty(ty.clone());
}
self.visit_middle_ty(fn_sig.output().skip_binder().clone());
}
}

impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
Expand Down Expand Up @@ -618,6 +664,19 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
intravisit::walk_struct_def(self, def);
}

fn visit_field_def(&mut self, s: &'tcx rustc_hir::FieldDef<'tcx>) {
// check `field: T::Ty`
// marks assoc types live whether the field is not used or not
// there are three situations:
// 1. the field is used, it's good
// 2. the field is not used but marked like `#[allow(dead_code)]`,
// it's annoying to mark the assoc type `#[allow(dead_code)]` again
// 3. the field is not used, and will be linted
// the assoc type will be linted after removing the unused field
self.visit_middle_ty_by_def_id(s.def_id);
intravisit::walk_field_def(self, s);
}

fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
match expr.kind {
hir::ExprKind::Path(ref qpath @ hir::QPath::TypeRelative(..)) => {
Expand Down Expand Up @@ -650,6 +709,9 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
_ => (),
}

// check the expr_ty if its type is `T::Ty`
self.visit_middle_ty(self.typeck_results().expr_ty(expr));

intravisit::walk_expr(self, expr);
}

Expand Down Expand Up @@ -722,6 +784,41 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {

self.in_pat = in_pat;
}

fn visit_poly_trait_ref(&mut self, t: &'tcx hir::PolyTraitRef<'tcx>) {
// mark the assoc type/const appears in poly-trait-ref live
if let Some(pathsegment) = t.trait_ref.path.segments.last()
&& let Some(args) = pathsegment.args
{
for constraint in args.constraints {
if let Some(item) = self
.tcx
.associated_items(pathsegment.res.def_id())
.filter_by_name_unhygienic(constraint.ident.name)
.find(|i| {
matches!(i.kind, ty::AssocKind::Const | ty::AssocKind::Type)
&& i.ident(self.tcx).normalize_to_macros_2_0() == constraint.ident
})
&& let Some(local_def_id) = item.def_id.as_local()
{
self.worklist.push((local_def_id, ComesFromAllowExpect::No));
}
}
}
intravisit::walk_poly_trait_ref(self, t);
}
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for MarkSymbolVisitor<'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) {
match ty.kind() {
ty::Alias(_, alias) => {
self.check_def_id(alias.def_id);
}
_ => (),
}
ty.super_visit_with(self);
}
}

fn has_allow_dead_code_or_lang_attr(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use crate::layout;
pub(crate) trait QueryContext {
type Def: layout::Def;
type Ref: layout::Ref;
type Scope: Copy;
}

#[cfg(test)]
Expand All @@ -28,19 +27,16 @@ pub(crate) mod test {
impl QueryContext for UltraMinimal {
type Def = Def;
type Ref = !;
type Scope = ();
}
}

#[cfg(feature = "rustc")]
mod rustc {
use super::*;
use rustc_middle::ty::{Ty, TyCtxt};
use rustc_middle::ty::TyCtxt;

impl<'tcx> super::QueryContext for TyCtxt<'tcx> {
type Def = layout::rustc::Def<'tcx>;
type Ref = layout::rustc::Ref<'tcx>;

type Scope = Ty<'tcx>;
}
}
1 change: 1 addition & 0 deletions library/alloc/src/vec/in_place_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ const fn needs_realloc<SRC, DEST>(src_cap: usize, dst_cap: usize) -> bool {

/// This provides a shorthand for the source type since local type aliases aren't a thing.
#[rustc_specialization_trait]
#[allow(dead_code)]
trait InPlaceCollect: SourceIter<Source: AsVecIntoIter> + InPlaceIterable {
type Src;
}
Expand Down
1 change: 1 addition & 0 deletions library/core/src/ops/async_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ mod internal_implementation_detail {
// of the closure's self-capture, and these upvar types will be instantiated with
// the `'closure_env` region provided to the associated type.
#[lang = "async_fn_kind_upvars"]
#[allow(dead_code)]
type Upvars<'closure_env, Inputs, Upvars, BorrowedUpvarsAsFnPtr>;
}
}
2 changes: 1 addition & 1 deletion tests/ui/associated-type-bounds/union-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

trait Tr1: Copy { type As1: Copy; }
trait Tr2: Copy { type As2: Copy; }
trait Tr3: Copy { type As3: Copy; }
trait Tr3: Copy { #[allow(dead_code)] type As3: Copy; }
trait Tr4<'a>: Copy { type As4: Copy; }
trait Tr5: Copy { type As5: Copy; }

Expand Down
1 change: 1 addition & 0 deletions tests/ui/associated-types/impl-wf-cycle-5.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ impl Fiz for bool {}

trait Grault {
type A;
#[allow(dead_code)]
type B;
}

Expand Down
1 change: 1 addition & 0 deletions tests/ui/associated-types/impl-wf-cycle-5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ impl Fiz for bool {}

trait Grault {
type A;
#[allow(dead_code)]
type B;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/associated-types/impl-wf-cycle-5.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-5.rs:22:1
--> $DIR/impl-wf-cycle-5.rs:23:1
|
LL | / impl<T> Grault for (T,)
LL | |
Expand All @@ -12,7 +12,7 @@ LL | type A = ();
| ------ associated type `<(T,) as Grault>::A` is specified here
|
note: required for `(T,)` to implement `Grault`
--> $DIR/impl-wf-cycle-5.rs:22:9
--> $DIR/impl-wf-cycle-5.rs:23:9
|
LL | impl<T> Grault for (T,)
| ^^^^^^ ^^^^
Expand Down
1 change: 1 addition & 0 deletions tests/ui/associated-types/impl-wf-cycle-6.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ impl Fiz for bool {}

trait Grault {
type A;
#[allow(dead_code)]
type B;
}

Expand Down
1 change: 1 addition & 0 deletions tests/ui/associated-types/impl-wf-cycle-6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ impl Fiz for bool {}

trait Grault {
type A;
#[allow(dead_code)]
type B;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/associated-types/impl-wf-cycle-6.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0275]: overflow evaluating the requirement `<(T,) as Grault>::A == _`
--> $DIR/impl-wf-cycle-6.rs:22:1
--> $DIR/impl-wf-cycle-6.rs:23:1
|
LL | / impl<T: Grault> Grault for (T,)
LL | |
Expand All @@ -11,7 +11,7 @@ LL | type A = ();
| ------ associated type `<(T,) as Grault>::A` is specified here
|
note: required for `(T,)` to implement `Grault`
--> $DIR/impl-wf-cycle-6.rs:22:17
--> $DIR/impl-wf-cycle-6.rs:23:17
|
LL | impl<T: Grault> Grault for (T,)
| ^^^^^^ ^^^^
Expand Down
1 change: 1 addition & 0 deletions tests/ui/generic-associated-types/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ trait Collection<T> {
type Iter<'iter>: Iterator<Item=&'iter T> where T: 'iter, Self: 'iter;
type Family: CollectionFamily;
// Test associated type defaults with parameters
#[allow(dead_code)]
type Sibling<U>: Collection<U> =
<<Self as Collection<T>>::Family as CollectionFamily>::Member<U>;

Expand Down
23 changes: 23 additions & 0 deletions tests/ui/lint/dead-code/unused-assoc-ty-in-used-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![deny(dead_code)]

trait Tr {
type X; //~ ERROR associated type `X` is never used
type Y;
}

impl Tr for () {
type X = Self;
type Y = Self;
}

trait Tr2 {
type X;
}

fn foo() -> impl Tr<Y = ()> {}
fn bar<T: ?Sized>() {}

fn main() {
foo();
bar::<dyn Tr2<X = ()>>();
}
16 changes: 16 additions & 0 deletions tests/ui/lint/dead-code/unused-assoc-ty-in-used-trait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: associated type `X` is never used
--> $DIR/unused-assoc-ty-in-used-trait.rs:4:10
|
LL | trait Tr {
| -- associated type in this trait
LL | type X;
| ^
|
note: the lint level is defined here
--> $DIR/unused-assoc-ty-in-used-trait.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^

error: aborting due to 1 previous error

3 changes: 2 additions & 1 deletion tests/ui/traits/issue-38033.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ trait Future {

trait IntoFuture {
type Future: Future<Item=Self::Item, Error=Self::Error>;
//~^ WARN associated items `Future` and `into_future` are never used
type Item;
type Error;

fn into_future(self) -> Self::Future; //~ WARN method `into_future` is never used
fn into_future(self) -> Self::Future;
}

impl<F: Future> IntoFuture for F {
Expand Down
8 changes: 5 additions & 3 deletions tests/ui/traits/issue-38033.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
warning: method `into_future` is never used
--> $DIR/issue-38033.rs:22:8
warning: associated items `Future` and `into_future` are never used
--> $DIR/issue-38033.rs:18:10
|
LL | trait IntoFuture {
| ---------- method in this trait
| ---------- associated items in this trait
LL | type Future: Future<Item=Self::Item, Error=Self::Error>;
| ^^^^^^
...
LL | fn into_future(self) -> Self::Future;
| ^^^^^^^^^^^
Expand Down

0 comments on commit a3531fb

Please sign in to comment.