Skip to content

Lint unused assoc tys although the trait is used #127714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 123 additions & 8 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,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 @@ -465,7 +465,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 @@ -478,12 +478,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 @@ -519,6 +520,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 @@ -540,6 +556,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 @@ -600,6 +630,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 @@ -631,6 +677,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 @@ -663,6 +722,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 @@ -703,6 +765,24 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {

fn visit_path(&mut self, path: &hir::Path<'tcx>, _: hir::HirId) {
self.handle_res(path.res);

if let Res::Def(def_kind, def_id) = path.res
&& matches!(
def_kind,
DefKind::Fn
| DefKind::AssocFn
| DefKind::AssocTy
| DefKind::Struct
| DefKind::Union
| DefKind::Enum
)
{
let preds = self.tcx.predicates_of(def_id).instantiate_identity(self.tcx);
for pred in preds.iter() {
<Self as TypeVisitor<TyCtxt<'tcx>>>::visit_predicate(self, pred.0.as_predicate());
}
}

intravisit::walk_path(self, path);
}

Expand Down Expand Up @@ -735,6 +815,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,20 +27,17 @@ pub(crate) mod test {
impl QueryContext for UltraMinimal {
type Def = Def;
type Ref = !;
type Scope = ();
}
}

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

use super::*;

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 @@ -213,6 +213,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
25 changes: 25 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,25 @@
#![deny(dead_code)]

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

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

trait Tr2 {
type X;
}

fn foo<T: Tr>() -> impl Tr<Y = ()> where T::Z: Copy {}
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
Loading