Skip to content

Commit

Permalink
Improve dead code analysis
Browse files Browse the repository at this point in the history
1. check impl items later if self ty is private although the trait method is public, cause we must use the ty firstly if it's private
2. mark the adt live if it appears in pattern, like generic argument, this implies the use of the adt
3. based on the above, we can handle the case that private adts impl Default, so that we don't need adding rustc_trivial_field_reads on Default, and the logic in should_ignore_item
  • Loading branch information
mu001999 committed Jun 29, 2024
1 parent bae813a commit 218acea
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 52 deletions.
40 changes: 12 additions & 28 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
pats: &[hir::PatField<'_>],
) {
let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
ty::Adt(adt, _) => adt.variant_of_res(res),
ty::Adt(adt, _) => {
self.check_def_id(adt.did());
adt.variant_of_res(res)
}
_ => span_bug!(lhs.span, "non-ADT in struct pattern"),
};
for pat in pats {
Expand All @@ -297,7 +300,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
dotdot: hir::DotDotPos,
) {
let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
ty::Adt(adt, _) => adt.variant_of_res(res),
ty::Adt(adt, _) => {
self.check_def_id(adt.did());
adt.variant_of_res(res)
}
_ => {
self.tcx.dcx().span_delayed_bug(lhs.span, "non-ADT in tuple struct pattern");
return;
Expand Down Expand Up @@ -402,31 +408,6 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
return false;
}

// don't ignore impls for Enums and pub Structs whose methods don't have self receiver,
// cause external crate may call such methods to construct values of these types
if let Some(local_impl_of) = impl_of.as_local()
&& let Some(local_def_id) = def_id.as_local()
&& let Some(fn_sig) =
self.tcx.hir().fn_sig_by_hir_id(self.tcx.local_def_id_to_hir_id(local_def_id))
&& matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None)
&& let TyKind::Path(hir::QPath::Resolved(_, path)) =
self.tcx.hir().expect_item(local_impl_of).expect_impl().self_ty.kind
&& let Res::Def(def_kind, did) = path.res
{
match def_kind {
// for example, #[derive(Default)] pub struct T(i32);
// external crate can call T::default() to construct T,
// so that don't ignore impl Default for pub Enum and Structs
DefKind::Struct | DefKind::Union if self.tcx.visibility(did).is_public() => {
return false;
}
// don't ignore impl Default for Enums,
// cause we don't know which variant is constructed
DefKind::Enum => return false,
_ => (),
};
}

if let Some(trait_of) = self.tcx.trait_id_of_impl(impl_of)
&& self.tcx.has_attr(trait_of, sym::rustc_trivial_field_reads)
{
Expand Down Expand Up @@ -690,6 +671,9 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
self.handle_field_pattern_match(pat, res, fields);
}
PatKind::Path(ref qpath) => {
if let ty::Adt(adt, _) = self.typeck_results().node_type(pat.hir_id).kind() {
self.check_def_id(adt.did());
}
let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
self.handle_res(res);
}
Expand Down Expand Up @@ -845,7 +829,7 @@ fn check_item<'tcx>(
// mark the method live if the self_ty is public,
// or the method is public and may construct self
if tcx.visibility(local_def_id).is_public()
&& (ty_and_all_fields_are_public || may_construct_self)
&& (ty_and_all_fields_are_public || (ty_is_public && may_construct_self))
{
// if the impl item is public,
// and the ty may be constructed or can be constructed in foreign crates,
Expand Down
1 change: 0 additions & 1 deletion library/core/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ use crate::ascii::Char as AsciiChar;
/// ```
#[cfg_attr(not(test), rustc_diagnostic_item = "Default")]
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(bootstrap), rustc_trivial_field_reads)]
pub trait Default: Sized {
/// Returns the "default value" for a type.
///
Expand Down
1 change: 1 addition & 0 deletions tests/ui/const-generics/issues/issue-86535-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub trait Foo {
fn foo() where [(); Self::ASSOC_C]:;
}

#[allow(dead_code)]
struct Bar<const N: &'static ()>;
impl<const N: &'static ()> Foo for Bar<N> {
const ASSOC_C: usize = 3;
Expand Down
1 change: 1 addition & 0 deletions tests/ui/const-generics/issues/issue-86535.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![feature(adt_const_params, generic_const_exprs)]
#![allow(incomplete_features, unused_variables)]

#[allow(dead_code)]
struct F<const S: &'static str>;
impl<const S: &'static str> X for F<{ S }> {
const W: usize = 3;
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/impl-trait/extra-impl-in-trait-impl.fixed
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//@ run-rustfix

#[allow(dead_code)]
struct S<T>(T);
#[allow(dead_code)]
struct S2;

impl<T: Default> Default for S<T> {
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/impl-trait/extra-impl-in-trait-impl.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//@ run-rustfix

#[allow(dead_code)]
struct S<T>(T);
#[allow(dead_code)]
struct S2;

impl<T: Default> impl Default for S<T> {
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/impl-trait/extra-impl-in-trait-impl.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
error: unexpected `impl` keyword
--> $DIR/extra-impl-in-trait-impl.rs:6:18
--> $DIR/extra-impl-in-trait-impl.rs:8:18
|
LL | impl<T: Default> impl Default for S<T> {
| ^^^^^ help: remove the extra `impl`
|
note: this is parsed as an `impl Trait` type, but a trait is expected at this position
--> $DIR/extra-impl-in-trait-impl.rs:6:18
--> $DIR/extra-impl-in-trait-impl.rs:8:18
|
LL | impl<T: Default> impl Default for S<T> {
| ^^^^^^^^^^^^

error: unexpected `impl` keyword
--> $DIR/extra-impl-in-trait-impl.rs:12:6
--> $DIR/extra-impl-in-trait-impl.rs:14:6
|
LL | impl impl Default for S2 {
| ^^^^^ help: remove the extra `impl`
|
note: this is parsed as an `impl Trait` type, but a trait is expected at this position
--> $DIR/extra-impl-in-trait-impl.rs:12:6
--> $DIR/extra-impl-in-trait-impl.rs:14:6
|
LL | impl impl Default for S2 {
| ^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/lint/dead-code/issue-59003.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#![deny(dead_code)]

#[allow(dead_code)]
struct Foo {
#[allow(dead_code)]
inner: u32,
}

Expand Down
32 changes: 32 additions & 0 deletions tests/ui/lint/dead-code/not-lint-adt-appeared-in-pattern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//@ check-pass

#![deny(dead_code)]

#[repr(u8)]
#[derive(Copy, Clone, Debug)]
pub enum RecordField {
Target = 1,
Level,
Module,
File,
Line,
NumArgs,
}

unsafe trait Pod {}

#[repr(transparent)]
struct RecordFieldWrapper(RecordField);

unsafe impl Pod for RecordFieldWrapper {}

fn try_read<T: Pod>(buf: &[u8]) -> T {
unsafe { std::ptr::read_unaligned(buf.as_ptr() as *const T) }
}

pub fn foo(buf: &[u8]) -> RecordField {
let RecordFieldWrapper(tag) = try_read(buf);
tag
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
#![deny(dead_code)]

struct T1; //~ ERROR struct `T1` is never constructed
pub struct T2(i32); //~ ERROR struct `T2` is never constructed
struct T3;
struct T2; //~ ERROR struct `T2` is never constructed
pub struct T3(i32); //~ ERROR struct `T3` is never constructed
pub struct T4(i32); //~ ERROR field `0` is never read

trait Trait1 { //~ ERROR trait `Trait1` is never used
const UNUSED: i32;
Expand All @@ -11,13 +12,13 @@ trait Trait1 { //~ ERROR trait `Trait1` is never used
}

pub trait Trait2 {
const USED: i32;
fn used(&self) {}
const MAY_USED: i32;
fn may_used(&self) {}
}

pub trait Trait3 {
const USED: i32;
fn construct_self() -> Self;
const MAY_USED: i32;
fn may_used() -> Self;
}

impl Trait1 for T1 {
Expand All @@ -30,23 +31,34 @@ impl Trait1 for T1 {
impl Trait1 for T2 {
const UNUSED: i32 = 0;
fn construct_self() -> Self {
T2(0)
Self
}
}

impl Trait2 for T1 {
const USED: i32 = 0;
const MAY_USED: i32 = 0;
}

impl Trait2 for T2 {
const USED: i32 = 0;
const MAY_USED: i32 = 0;
}

impl Trait3 for T3 {
const USED: i32 = 0;
fn construct_self() -> Self {
impl Trait2 for T3 {
const MAY_USED: i32 = 0;
}

impl Trait3 for T2 {
const MAY_USED: i32 = 0;
fn may_used() -> Self {
Self
}
}

impl Trait3 for T4 {
const MAY_USED: i32 = 0;
fn may_used() -> Self {
T4(0)
}
}

fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,32 @@ LL | #![deny(dead_code)]
| ^^^^^^^^^

error: struct `T2` is never constructed
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:4:12
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:4:8
|
LL | pub struct T2(i32);
LL | struct T2;
| ^^

error: struct `T3` is never constructed
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:5:12
|
LL | pub struct T3(i32);
| ^^

error: field `0` is never read
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:6:15
|
LL | pub struct T4(i32);
| -- ^^^
| |
| field in this struct
|
= help: consider removing this field

error: trait `Trait1` is never used
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:7:7
--> $DIR/unused-adt-impl-pub-trait-with-assoc-const.rs:8:7
|
LL | trait Trait1 {
| ^^^^^^

error: aborting due to 3 previous errors
error: aborting due to 5 previous errors

1 change: 1 addition & 0 deletions tests/ui/lint/dead-code/unused-struct-derive-default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ pub struct T2 {

fn main() {
let _x: Used = Default::default();
let _e: E = Default::default();
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ error: struct `T` is never constructed
LL | struct T;
| ^
|
= note: `T` has a derived impl for the trait `Default`, but this is intentionally ignored during dead code analysis
note: the lint level is defined here
--> $DIR/unused-struct-derive-default.rs:1:9
|
Expand Down
1 change: 1 addition & 0 deletions tests/ui/parser/issues/issue-105366.fixed
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ run-rustfix

#[allow(dead_code)]
struct Foo;

impl From<i32> for Foo {
Expand Down
1 change: 1 addition & 0 deletions tests/ui/parser/issues/issue-105366.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ run-rustfix

#[allow(dead_code)]
struct Foo;

fn From<i32> for Foo {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/parser/issues/issue-105366.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: you might have meant to write `impl` instead of `fn`
--> $DIR/issue-105366.rs:5:1
--> $DIR/issue-105366.rs:6:1
|
LL | fn From<i32> for Foo {
| ^^
Expand Down

0 comments on commit 218acea

Please sign in to comment.