Skip to content

Commit

Permalink
Rollup merge of rust-lang#118257 - mu001999:dead_code/trait, r=cjgillot
Browse files Browse the repository at this point in the history
Make traits / trait methods detected by the dead code lint

Fixes rust-lang#118139 and rust-lang#41883
  • Loading branch information
GuillaumeGomez authored Jan 20, 2024
2 parents 6745c60 + 155ca3d commit c326059
Show file tree
Hide file tree
Showing 269 changed files with 1,167 additions and 250 deletions.
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 @@ -632,10 +670,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 @@ -644,7 +678,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 @@ -675,7 +713,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 @@ -944,7 +982,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 @@ -969,18 +1008,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 @@ -993,7 +1047,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 @@ -1040,8 +1093,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
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
@@ -1,28 +1,34 @@
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 @@ -380,14 +380,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
4 changes: 2 additions & 2 deletions src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use std::path::{Path, PathBuf};

const ENTRY_LIMIT: usize = 900;
// FIXME: The following limits should be reduced eventually.
const ISSUES_ENTRY_LIMIT: usize = 1849;
const ROOT_ENTRY_LIMIT: usize = 870;
const ISSUES_ENTRY_LIMIT: usize = 1861;
const ROOT_ENTRY_LIMIT: usize = 872;

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
6 changes: 3 additions & 3 deletions tests/ui/anon-params/anon-params-deprecated.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: anonymous parameters are deprecated and will be removed in the next edition
--> $DIR/anon-params-deprecated.rs:9:12
--> $DIR/anon-params-deprecated.rs:10:12
|
LL | fn foo(i32);
| ^^^ help: try naming the parameter or explicitly ignoring it: `_: i32`
Expand All @@ -13,7 +13,7 @@ LL | #![warn(anonymous_parameters)]
| ^^^^^^^^^^^^^^^^^^^^

warning: anonymous parameters are deprecated and will be removed in the next edition
--> $DIR/anon-params-deprecated.rs:12:30
--> $DIR/anon-params-deprecated.rs:13:30
|
LL | fn bar_with_default_impl(String, String) {}
| ^^^^^^ help: try naming the parameter or explicitly ignoring it: `_: String`
Expand All @@ -22,7 +22,7 @@ LL | fn bar_with_default_impl(String, String) {}
= note: for more information, see issue #41686 <https://github.com/rust-lang/rust/issues/41686>

warning: anonymous parameters are deprecated and will be removed in the next edition
--> $DIR/anon-params-deprecated.rs:12:38
--> $DIR/anon-params-deprecated.rs:13:38
|
LL | fn bar_with_default_impl(String, String) {}
| ^^^^^^ help: try naming the parameter or explicitly ignoring it: `_: String`
Expand Down
Loading

0 comments on commit c326059

Please sign in to comment.