Skip to content

Commit

Permalink
Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r…
Browse files Browse the repository at this point in the history
…=pnkfelix

Improve dead code analysis

Fixes rust-lang#120770

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

r? ``@pnkfelix``
  • Loading branch information
compiler-errors authored Jul 6, 2024
2 parents f203078 + 0adb825 commit 31fe962
Show file tree
Hide file tree
Showing 21 changed files with 164 additions and 60 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
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,3 @@ impl Parse for QueryGroup {
Ok(QueryGroup { group_path })
}
}

struct Nothing;

impl Parse for Nothing {
fn parse(_input: ParseStream<'_>) -> syn::Result<Self> {
Ok(Nothing)
}
}
3 changes: 3 additions & 0 deletions tests/ui-fulldeps/deriving-global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,21 @@ mod submod {
// if any of these are implemented without global calls for any
// function calls, then being in a submodule will (correctly)
// cause errors about unrecognised module `std` (or `extra`)
#[allow(dead_code)]
#[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Clone, Debug, Encodable, Decodable)]
enum A {
A1(usize),
A2(isize),
}

#[allow(dead_code)]
#[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Clone, Debug, Encodable, Decodable)]
struct B {
x: usize,
y: isize,
}

#[allow(dead_code)]
#[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Clone, Debug, Encodable, Decodable)]
struct C(usize, isize);
}
Expand Down
1 change: 1 addition & 0 deletions tests/ui-fulldeps/deriving-hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub const s: u8 = 1;
pub const state: u8 = 1;
pub const cmp: u8 = 1;

#[allow(dead_code)]
#[derive(Ord, Eq, PartialOrd, PartialEq, Debug, Decodable, Encodable, Hash)]
struct Foo {}

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
37 changes: 37 additions & 0 deletions tests/ui/lint/dead-code/lint-unused-adt-appeared-in-pattern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#![deny(dead_code)]

struct Foo(u8); //~ ERROR struct `Foo` is never constructed

enum Bar { //~ ERROR enum `Bar` is never used
Var1(u8),
Var2(u8),
}

pub trait Tr1 {
fn f1() -> Self;
}

impl Tr1 for Foo {
fn f1() -> Foo {
let f = Foo(0);
let Foo(tag) = f;
Foo(tag)
}
}

impl Tr1 for Bar {
fn f1() -> Bar {
let b = Bar::Var1(0);
let b = if let Bar::Var1(_) = b {
Bar::Var1(0)
} else {
Bar::Var2(0)
};
match b {
Bar::Var1(_) => Bar::Var2(0),
Bar::Var2(_) => Bar::Var1(0),
}
}
}

fn main() {}
20 changes: 20 additions & 0 deletions tests/ui/lint/dead-code/lint-unused-adt-appeared-in-pattern.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: struct `Foo` is never constructed
--> $DIR/lint-unused-adt-appeared-in-pattern.rs:3:8
|
LL | struct Foo(u8);
| ^^^
|
note: the lint level is defined here
--> $DIR/lint-unused-adt-appeared-in-pattern.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^

error: enum `Bar` is never used
--> $DIR/lint-unused-adt-appeared-in-pattern.rs:5:6
|
LL | enum Bar {
| ^^^

error: aborting due to 2 previous errors

32 changes: 32 additions & 0 deletions tests/ui/lint/dead-code/not-lint-used-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

Loading

0 comments on commit 31fe962

Please sign in to comment.