Skip to content

Commit

Permalink
Detect unused structs which implement private traits
Browse files Browse the repository at this point in the history
  • Loading branch information
mu001999 committed Mar 12, 2024
1 parent c69fda7 commit e48bbd3
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 17 deletions.
26 changes: 26 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,32 @@ rustc_queries! {
desc { |tcx| "comparing impl items against trait for `{}`", tcx.def_path_str(impl_id) }
}

/// Maps from associated items on the impl specified by `impl_id`
/// to the corresponding associated item on the trait.
///
/// For example, with the following code
///
/// ```
/// struct Type {}
/// // DefId
/// trait Trait { // trait_id
/// fn f(); // trait_f
/// fn g() {} // trait_g
/// }
///
/// impl Trait for Type { // impl_id
/// fn f() {} // impl_f
/// fn g() {} // impl_g
/// }
/// ```
///
/// The map returned for `tcx.impl_item_trait_item_ids(impl_id)` would be
///`{ impl_f: trait_f, impl_g: trait_g }`
query impl_item_trait_item_ids(impl_id: DefId) -> &'tcx DefIdMap<DefId> {
arena_cache
desc { |tcx| "comparing impl items against trait for `{}`", tcx.def_path_str(impl_id) }
}

/// Given `fn_def_id` of a trait or of an impl that implements a given trait:
/// if `fn_def_id` is the def id of a function defined inside a trait, then it creates and returns
/// the associated items that correspond to each impl trait in return position for that trait.
Expand Down
54 changes: 38 additions & 16 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
&& let ItemKind::Impl(impl_ref) =
self.tcx.hir().expect_item(local_impl_id).kind
{
if self.tcx.visibility(trait_id).is_public()
&& matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
if matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
&& !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;
}

Expand Down Expand Up @@ -487,32 +488,51 @@ 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, _)| self.impl_item_with_used_self(impl_id));
(ready, unsolved_impl_items) =
unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| {
self.impl_item_with_used_self_and_trait_term(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, _)| self.impl_item_with_used_self(impl_id));
(ready, unsolved_impl_items) =
unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| {
self.impl_item_with_used_self_and_trait_term(impl_id, impl_item_id)
});
}
}

fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId) -> bool {
fn impl_item_with_used_self_and_trait_term(
&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)
{
self.live_symbols.contains(&local_def_id)
} else {
false
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);
} else if let Some(trait_item_id) = self
.tcx
.impl_item_trait_item_ids(impl_id.owner_id.to_def_id())
.get(&impl_item_id.to_def_id())
&& let Some(local_id) = trait_item_id.as_local()
{
// for the private method, we can know the trait item is ued 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);
}
}
false
}
}

Expand Down Expand Up @@ -746,20 +766,22 @@ fn check_item<'tcx>(
matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None);
}

// for impl trait blocks, mark associate functions live if the trait is public
// 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 of_trait && tcx.visibility(local_def_id).is_public() {
// pub method && private ty & methods not construct self
unsolved_impl_items.push((id, local_def_id));
} else 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));
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_ty_utils/src/assoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub(crate) fn provide(providers: &mut Providers) {
associated_types_for_impl_traits_in_associated_fn,
associated_type_for_impl_trait_in_trait,
impl_item_implementor_ids,
impl_item_trait_item_ids,
..*providers
};
}
Expand Down Expand Up @@ -92,6 +93,13 @@ fn impl_item_implementor_ids(tcx: TyCtxt<'_>, impl_id: DefId) -> DefIdMap<DefId>
.collect()
}

fn impl_item_trait_item_ids(tcx: TyCtxt<'_>, impl_id: DefId) -> DefIdMap<DefId> {
tcx.associated_items(impl_id)
.in_definition_order()
.filter_map(|item| item.trait_item_def_id.map(|trait_item| (item.def_id, trait_item)))
.collect()
}

fn associated_item(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::AssocItem {
let id = tcx.local_def_id_to_hir_id(def_id);
let parent_def_id = tcx.hir().get_parent_item(id);
Expand Down
34 changes: 34 additions & 0 deletions tests/ui/lint/dead-code/unused-adt-impls-trait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![deny(dead_code)]

struct Used;
struct Unused; //~ ERROR struct `Unused` is never constructed

pub trait PubTrait {
fn foo(&self) -> Self;
}

impl PubTrait for Used {
fn foo(&self) -> Self { Used }
}

impl PubTrait for Unused {
fn foo(&self) -> Self { Unused }
}

trait PriTrait {
fn foo(&self) -> Self;
}

impl PriTrait for Used {
fn foo(&self) -> Self { Used }
}

impl PriTrait for Unused {
fn foo(&self) -> Self { Unused }
}

fn main() {
let t = Used;
let _t = <Used as PubTrait>::foo(&t);
let _t = <Used as PriTrait>::foo(&t);
}
14 changes: 14 additions & 0 deletions tests/ui/lint/dead-code/unused-adt-impls-trait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: struct `Unused` is never constructed
--> $DIR/unused-adt-impls-trait.rs:4:8
|
LL | struct Unused;
| ^^^^^^
|
note: the lint level is defined here
--> $DIR/unused-adt-impls-trait.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^

error: aborting due to 1 previous error

1 change: 1 addition & 0 deletions tests/ui/rust-2021/inherent-dyn-collision.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod inner {
// having a struct of the same name as the trait in-scope, while *also*
// implementing the trait for that struct but **without** importing the
// trait itself into scope
#[allow(dead_code)]
struct TryIntoU32;

impl super::TryIntoU32 for TryIntoU32 {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/rust-2021/inherent-dyn-collision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ mod inner {
// having a struct of the same name as the trait in-scope, while *also*
// implementing the trait for that struct but **without** importing the
// trait itself into scope
#[allow(dead_code)]
struct TryIntoU32;

impl super::TryIntoU32 for TryIntoU32 {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/rust-2021/inherent-dyn-collision.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: trait method `try_into` will become ambiguous in Rust 2021
--> $DIR/inherent-dyn-collision.rs:41:9
--> $DIR/inherent-dyn-collision.rs:42:9
|
LL | get_dyn_trait().try_into().unwrap()
| ^^^^^^^^^^^^^^^ help: disambiguate the method call: `(&*get_dyn_trait())`
Expand Down

0 comments on commit e48bbd3

Please sign in to comment.