Skip to content

Make traits / trait methods detected by the dead code lint #118257

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

Merged
merged 2 commits into from
Feb 7, 2024
Merged
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
91 changes: 71 additions & 20 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
// is dead.

use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
use hir::ItemKind;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::MultiSpan;
use rustc_hir as hir;
Expand All @@ -14,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, TyCtxt};
use rustc_middle::ty::{self, TyCtxt, Visibility};
use rustc_session::lint;
use rustc_session::lint::builtin::DEAD_CODE;
use rustc_span::symbol::{sym, Symbol};
Expand Down Expand Up @@ -381,9 +382,46 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
intravisit::walk_item(self, item)
}
hir::ItemKind::ForeignMod { .. } => {}
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) =
self.tcx.hir().expect_item(local_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);
}
}

intravisit::walk_item(self, item)
}
_ => intravisit::walk_item(self, item),
},
Node::TraitItem(trait_item) => {
// mark corresponing ImplTerm 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
{
// mark self_ty live
intravisit::walk_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);
}
Node::ImplItem(impl_item) => {
Expand Down Expand Up @@ -636,10 +674,6 @@ fn check_item<'tcx>(
}
}
DefKind::Impl { of_trait } => {
if of_trait {
worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No));
}

// get DefIds from another query
let local_def_ids = tcx
.associated_item_def_ids(id.owner_id)
Expand All @@ -648,7 +682,11 @@ fn check_item<'tcx>(

// And we access the Map here to get HirId from LocalDefId
for id in local_def_ids {
if of_trait {
// for impl trait blocks, mark associate functions live if the trait is public
if of_trait
&& (!matches!(tcx.def_kind(id), DefKind::AssocFn)
|| tcx.local_visibility(id) == Visibility::Public)
{
worklist.push((id, ComesFromAllowExpect::No));
} else if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, id) {
worklist.push((id, comes_from_allow));
Expand Down Expand Up @@ -679,7 +717,7 @@ fn check_trait_item(
use hir::TraitItemKind::{Const, Fn};
if matches!(tcx.def_kind(id.owner_id), DefKind::AssocConst | DefKind::AssocFn) {
let trait_item = tcx.hir().trait_item(id);
if matches!(trait_item.kind, Const(_, Some(_)) | Fn(_, hir::TraitFn::Provided(_)))
if matches!(trait_item.kind, Const(_, Some(_)) | Fn(..))
&& let Some(comes_from_allow) =
has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id)
{
Expand Down Expand Up @@ -948,7 +986,8 @@ impl<'tcx> DeadVisitor<'tcx> {
| DefKind::TyAlias
| DefKind::Enum
| DefKind::Union
| DefKind::ForeignTy => self.warn_dead_code(def_id, "used"),
| DefKind::ForeignTy
| DefKind::Trait => self.warn_dead_code(def_id, "used"),
DefKind::Struct => self.warn_dead_code(def_id, "constructed"),
DefKind::Variant | DefKind::Field => bug!("should be handled specially"),
_ => {}
Expand All @@ -973,18 +1012,33 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
let module_items = tcx.hir_module_items(module);

for item in module_items.items() {
if let hir::ItemKind::Impl(impl_item) = tcx.hir().item(item).kind {
let mut dead_items = Vec::new();
for item in impl_item.items {
let def_id = item.id.owner_id.def_id;
if !visitor.is_live_code(def_id) {
let name = tcx.item_name(def_id.to_def_id());
let level = visitor.def_lint_level(def_id);
let def_kind = tcx.def_kind(item.owner_id);

dead_items.push(DeadItem { def_id, name, level })
let mut dead_codes = Vec::new();
// if we have diagnosed the trait, do not diagnose unused methods
if matches!(def_kind, DefKind::Impl { .. })
|| (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)
{
let name = tcx.item_name(def_id);
let level = visitor.def_lint_level(local_def_id);
dead_codes.push(DeadItem { def_id: local_def_id, name, level });
}
}
visitor.warn_multiple(item.owner_id.def_id, "used", dead_items, ReportOn::NamedField);
}
if !dead_codes.is_empty() {
visitor.warn_multiple(item.owner_id.def_id, "used", dead_codes, ReportOn::NamedField);
}

if !live_symbols.contains(&item.owner_id.def_id) {
Expand All @@ -997,7 +1051,6 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
continue;
}

let def_kind = tcx.def_kind(item.owner_id);
if let DefKind::Struct | DefKind::Union | DefKind::Enum = def_kind {
let adt = tcx.adt_def(item.owner_id);
let mut dead_variants = Vec::new();
Expand Down Expand Up @@ -1044,8 +1097,6 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
for foreign_item in module_items.foreign_items() {
visitor.check_definition(foreign_item.owner_id.def_id);
}

// We do not warn trait items.
}

pub(crate) fn provide(providers: &mut Providers) {
Expand Down
49 changes: 2 additions & 47 deletions compiler/rustc_span/src/source_map/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use super::*;

use rustc_data_structures::sync::FreezeLock;

fn init_source_map() -> SourceMap {
let sm = SourceMap::new(FilePathMapping::empty());
sm.new_source_file(PathBuf::from("blork.rs").into(), "first line.\nsecond line".to_string());
Expand Down Expand Up @@ -263,53 +265,6 @@ fn t10() {
);
}

/// Returns the span corresponding to the `n`th occurrence of `substring` in `source_text`.
trait SourceMapExtension {
fn span_substr(
&self,
file: &Lrc<SourceFile>,
source_text: &str,
substring: &str,
n: usize,
) -> Span;
}

impl SourceMapExtension for SourceMap {
fn span_substr(
&self,
file: &Lrc<SourceFile>,
source_text: &str,
substring: &str,
n: usize,
) -> Span {
eprintln!(
"span_substr(file={:?}/{:?}, substring={:?}, n={})",
file.name, file.start_pos, substring, n
);
let mut i = 0;
let mut hi = 0;
loop {
let offset = source_text[hi..].find(substring).unwrap_or_else(|| {
panic!(
"source_text `{}` does not have {} occurrences of `{}`, only {}",
source_text, n, substring, i
);
});
let lo = hi + offset;
hi = lo + substring.len();
if i == n {
let span = Span::with_root_ctxt(
BytePos(lo as u32 + file.start_pos.0),
BytePos(hi as u32 + file.start_pos.0),
);
assert_eq!(&self.span_to_snippet(span).unwrap()[..], substring);
return span;
}
i += 1;
}
}
}

// Takes a unix-style path and returns a platform specific path.
fn path(p: &str) -> PathBuf {
path_str(p).into()
Expand Down
1 change: 1 addition & 0 deletions library/core/tests/macros.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[allow(dead_code)]
trait Trait {
fn blah(&self);
}
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys_common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ cfg_if::cfg_if! {

/// A trait for viewing representations from std types
#[doc(hidden)]
#[allow(dead_code)] // not used on all platforms
pub trait AsInner<Inner: ?Sized> {
fn as_inner(&self) -> &Inner;
}

/// A trait for viewing representations from std types
#[doc(hidden)]
#[allow(dead_code)] // not used on all platforms
pub trait AsInnerMut<Inner: ?Sized> {
fn as_inner_mut(&mut self) -> &mut Inner;
}
Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/tests/fail/dyn-call-trait-mismatch.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
trait T1 {
#[allow(dead_code)]
fn method1(self: Box<Self>);
}
trait T2 {
Expand Down
6 changes: 6 additions & 0 deletions src/tools/miri/tests/fail/dyn-upcast-trait-mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,36 @@
#![allow(incomplete_features)]

trait Foo: PartialEq<i32> + std::fmt::Debug + Send + Sync {
#[allow(dead_code)]
fn a(&self) -> i32 {
10
}

#[allow(dead_code)]
fn z(&self) -> i32 {
11
}

#[allow(dead_code)]
fn y(&self) -> i32 {
12
}
}

trait Bar: Foo {
#[allow(dead_code)]
fn b(&self) -> i32 {
20
}

#[allow(dead_code)]
fn w(&self) -> i32 {
21
}
}

trait Baz: Bar {
#[allow(dead_code)]
fn c(&self) -> i32 {
30
}
Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/tests/pass/cast-rfc0401-vtable-kinds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ trait Foo<T> {
}
}

#[allow(dead_code)]
trait Bar {
fn bar(&self) {
println!("Bar!");
Expand Down
3 changes: 3 additions & 0 deletions src/tools/miri/tests/pass/dyn-upcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,14 +383,17 @@ fn struct_() {

fn replace_vptr() {
trait A {
#[allow(dead_code)]
fn foo_a(&self);
}

trait B {
#[allow(dead_code)]
fn foo_b(&self);
}

trait C: A + B {
#[allow(dead_code)]
fn foo_c(&self);
}

Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/tests/pass/weak_memory/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::sync::atomic::Ordering::*;
use std::sync::atomic::{fence, AtomicUsize};
use std::thread::spawn;

#[allow(dead_code)]
#[derive(Copy, Clone)]
struct EvilSend<T>(pub T);

Expand Down
5 changes: 3 additions & 2 deletions src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ use std::path::{Path, PathBuf};
// desirable, because large numbers of files are unwieldy in general. See issue
// #73494.
const ENTRY_LIMIT: usize = 900;
const ISSUES_ENTRY_LIMIT: usize = 1807;
const ROOT_ENTRY_LIMIT: usize = 868;
// FIXME: The following limits should be reduced eventually.
const ISSUES_ENTRY_LIMIT: usize = 1819;
const ROOT_ENTRY_LIMIT: usize = 870;

const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
"rs", // test source files
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ fn start(_: isize, _: *const *const u8) -> isize {
//~ MONO_ITEM fn std::ptr::drop_in_place::<Struct<u32>> - shim(None) @@ instantiation_through_vtable-cgu.0[Internal]
//~ MONO_ITEM fn <Struct<u32> as Trait>::foo
//~ MONO_ITEM fn <Struct<u32> as Trait>::bar
let _ = &s1 as &Trait;
let r1 = &s1 as &Trait;
r1.foo();
r1.bar();

let s1 = Struct { _a: 0u64 };
//~ MONO_ITEM fn std::ptr::drop_in_place::<Struct<u64>> - shim(None) @@ instantiation_through_vtable-cgu.0[Internal]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,8 @@ fn start(_: isize, _: *const *const u8) -> isize {
//~ MONO_ITEM fn <u32 as SomeGenericTrait<i16>>::bar::<()>
0u32.bar(0i16, ());

0i8.foo();
0i32.foo();

0
}
2 changes: 2 additions & 0 deletions tests/codegen-units/item-collection/unsizing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,7 @@ fn start(_: isize, _: *const *const u8) -> isize {
//~ MONO_ITEM fn <u32 as Trait>::foo
let _wrapper_sized = wrapper_sized as Wrapper<Trait>;

false.foo();

0
}
2 changes: 1 addition & 1 deletion tests/ui-fulldeps/rustc_encodable_hygiene.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// run-pass
// check-pass

#![feature(rustc_private)]

Expand Down
1 change: 1 addition & 0 deletions tests/ui/anon-params/anon-params-deprecated.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// edition:2015
// run-rustfix

#[allow(dead_code)]
trait T {
fn foo(_: i32); //~ WARNING anonymous parameters are deprecated
//~| WARNING this is accepted in the current edition
Expand Down
1 change: 1 addition & 0 deletions tests/ui/anon-params/anon-params-deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// edition:2015
// run-rustfix

#[allow(dead_code)]
trait T {
fn foo(i32); //~ WARNING anonymous parameters are deprecated
//~| WARNING this is accepted in the current edition
Expand Down
Loading