Skip to content
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

Revert recent changes to dead code analysis (this time, stable) #128618

Closed
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
121 changes: 28 additions & 93 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,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, AssocItemContainer, TyCtxt};
use rustc_middle::ty::{self, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_session::lint::builtin::DEAD_CODE;
Expand Down Expand Up @@ -44,63 +44,16 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
)
}

struct Publicness {
ty_is_public: bool,
ty_and_all_fields_are_public: bool,
}

impl Publicness {
fn new(ty_is_public: bool, ty_and_all_fields_are_public: bool) -> Self {
Self { ty_is_public, ty_and_all_fields_are_public }
}
}

fn struct_all_fields_are_public(tcx: TyCtxt<'_>, id: DefId) -> bool {
// treat PhantomData and positional ZST as public,
// we don't want to lint types which only have them,
// cause it's a common way to use such types to check things like well-formedness
tcx.adt_def(id).all_fields().all(|field| {
let field_type = tcx.type_of(field.did).instantiate_identity();
if field_type.is_phantom_data() {
return true;
}
let is_positional = field.name.as_str().starts_with(|c: char| c.is_ascii_digit());
if is_positional
&& tcx
.layout_of(tcx.param_env(field.did).and(field_type))
.map_or(true, |layout| layout.is_zst())
{
return true;
}
field.vis.is_public()
})
}

/// check struct and its fields are public or not,
/// for enum and union, just check they are public,
/// and doesn't solve types like &T for now, just skip them
fn ty_ref_to_pub_struct(tcx: TyCtxt<'_>, ty: &hir::Ty<'_>) -> Publicness {
fn ty_ref_to_pub_struct(tcx: TyCtxt<'_>, ty: &hir::Ty<'_>) -> bool {
if let TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind
&& let Res::Def(def_kind, def_id) = path.res
&& def_id.is_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
return match def_kind {
DefKind::Enum | DefKind::Union => {
let ty_is_public = tcx.visibility(def_id).is_public();
Publicness::new(ty_is_public, ty_is_public)
}
DefKind::Struct => {
let ty_is_public = tcx.visibility(def_id).is_public();
Publicness::new(
ty_is_public,
ty_is_public && struct_all_fields_are_public(tcx, def_id),
)
}
_ => Publicness::new(true, true),
};
tcx.visibility(def_id).is_public()
} else {
true
}

Publicness::new(true, true)
}

/// Determine if a work from the worklist is coming from the a `#[allow]`
Expand Down Expand Up @@ -474,11 +427,9 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
{
if matches!(trait_item.kind, hir::TraitItemKind::Fn(..))
&& !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty)
.ty_and_all_fields_are_public
{
// skip impl-items of non pure pub ty,
// cause we don't know the ty is constructed or not,
// check these later in `solve_rest_impl_items`
// skip methods of private ty,
// they would be solved in `solve_rest_impl_items`
continue;
}

Expand Down Expand Up @@ -559,21 +510,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
&& let Some(local_def_id) = def_id.as_local()
&& matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union)
{
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);
}

if let Some(trait_item_id) = self.tcx.associated_item(impl_item_id).trait_item_def_id
&& let Some(local_id) = trait_item_id.as_local()
{
// for the local impl item, we can know the trait item is used or not,
// for the private method, we can know the trait item is used or not,
// so we mark the method live if the self is used and the trait item is used
self.live_symbols.contains(&local_id) && self.live_symbols.contains(&local_def_id)
} else {
// for the foreign method and inherent pub method,
// we don't know the trait item or the method is used or not,
// so we mark the method live if the self is used
self.live_symbols.contains(&local_def_id)
return self.live_symbols.contains(&local_id)
&& self.live_symbols.contains(&local_def_id);
}
} else {
false
}
false
}
}

Expand Down Expand Up @@ -795,9 +747,7 @@ fn check_item<'tcx>(
.iter()
.filter_map(|def_id| def_id.as_local());

let self_ty = tcx.hir().item(id).expect_impl().self_ty;
let Publicness { ty_is_public, ty_and_all_fields_are_public } =
ty_ref_to_pub_struct(tcx, self_ty);
let ty_is_pub = ty_ref_to_pub_struct(tcx, tcx.hir().item(id).expect_impl().self_ty);

// And we access the Map here to get HirId from LocalDefId
for local_def_id in local_def_ids {
Expand All @@ -813,20 +763,18 @@ fn check_item<'tcx>(
// 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::AssocTy)
|| tcx.visibility(local_def_id).is_public()
&& (ty_and_all_fields_are_public || 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))
{
// if the impl item is public,
// and the ty may be constructed or can be constructed in foreign crates,
// mark the impl item live
worklist.push((local_def_id, ComesFromAllowExpect::No));
} 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 || tcx.visibility(local_def_id).is_public() && ty_is_public {
// private impl items of traits || public impl items not constructs self
} else if of_trait {
// private method || public method not constructs self
unsolved_impl_items.push((id, local_def_id));
}
}
Expand Down Expand Up @@ -893,14 +841,6 @@ fn create_and_seed_worklist(
effective_vis
.is_public_at_level(Level::Reachable)
.then_some(id)
.filter(|&id|
// checks impls, impl-items and pub structs with all public fields later
match tcx.def_kind(id) {
DefKind::Impl { .. } => false,
DefKind::AssocConst | DefKind::AssocFn => !matches!(tcx.associated_item(id).container, AssocItemContainer::ImplContainer),
DefKind::Struct => struct_all_fields_are_public(tcx, id.to_def_id()) || has_allow_dead_code_or_lang_attr(tcx, id).is_some(),
_ => true
})
.map(|id| (id, ComesFromAllowExpect::No))
})
// Seed entry point
Expand Down Expand Up @@ -1173,15 +1113,10 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
|| (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 assoc consts and fns in traits
// We have diagnosed unused methods in traits
if matches!(def_kind, DefKind::Impl { of_trait: true })
&& matches!(tcx.def_kind(def_id), DefKind::AssocConst | DefKind::AssocFn)
// skip unused public inherent methods,
// cause we have diagnosed unconstructed struct
|| matches!(def_kind, DefKind::Impl { of_trait: false })
&& tcx.visibility(def_id).is_public()
&& ty_ref_to_pub_struct(tcx, tcx.hir().item(item).expect_impl().self_ty).ty_is_public
|| def_kind == DefKind::Trait && tcx.def_kind(def_id) == DefKind::AssocTy
&& tcx.def_kind(def_id) == DefKind::AssocFn
|| def_kind == DefKind::Trait && tcx.def_kind(def_id) != DefKind::AssocFn
{
continue;
}
Expand Down
10 changes: 5 additions & 5 deletions tests/codegen-units/item-collection/generic-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ impl<T> Struct<T> {
}
}

pub struct _LifeTimeOnly<'a> {
pub struct LifeTimeOnly<'a> {
_a: &'a u32,
}

impl<'a> _LifeTimeOnly<'a> {
//~ MONO_ITEM fn _LifeTimeOnly::<'_>::foo
impl<'a> LifeTimeOnly<'a> {
//~ MONO_ITEM fn LifeTimeOnly::<'_>::foo
pub fn foo(&self) {}
//~ MONO_ITEM fn _LifeTimeOnly::<'_>::bar
//~ MONO_ITEM fn LifeTimeOnly::<'_>::bar
pub fn bar(&'a self) {}
//~ MONO_ITEM fn _LifeTimeOnly::<'_>::baz
//~ MONO_ITEM fn LifeTimeOnly::<'_>::baz
pub fn baz<'b>(&'b self) {}

pub fn non_instantiated<T>(&self) {}
Expand Down
24 changes: 12 additions & 12 deletions tests/codegen-units/item-collection/overloaded-operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,44 @@

use std::ops::{Add, Deref, Index, IndexMut};

pub struct _Indexable {
pub struct Indexable {
data: [u8; 3],
}

impl Index<usize> for _Indexable {
impl Index<usize> for Indexable {
type Output = u8;

//~ MONO_ITEM fn <_Indexable as std::ops::Index<usize>>::index
//~ MONO_ITEM fn <Indexable as std::ops::Index<usize>>::index
fn index(&self, index: usize) -> &Self::Output {
if index >= 3 { &self.data[0] } else { &self.data[index] }
}
}

impl IndexMut<usize> for _Indexable {
//~ MONO_ITEM fn <_Indexable as std::ops::IndexMut<usize>>::index_mut
impl IndexMut<usize> for Indexable {
//~ MONO_ITEM fn <Indexable as std::ops::IndexMut<usize>>::index_mut
fn index_mut(&mut self, index: usize) -> &mut Self::Output {
if index >= 3 { &mut self.data[0] } else { &mut self.data[index] }
}
}

//~ MONO_ITEM fn <_Equatable as std::cmp::PartialEq>::eq
//~ MONO_ITEM fn <_Equatable as std::cmp::PartialEq>::ne
//~ MONO_ITEM fn <Equatable as std::cmp::PartialEq>::eq
//~ MONO_ITEM fn <Equatable as std::cmp::PartialEq>::ne
#[derive(PartialEq)]
pub struct _Equatable(u32);
pub struct Equatable(u32);

impl Add<u32> for _Equatable {
impl Add<u32> for Equatable {
type Output = u32;

//~ MONO_ITEM fn <_Equatable as std::ops::Add<u32>>::add
//~ MONO_ITEM fn <Equatable as std::ops::Add<u32>>::add
fn add(self, rhs: u32) -> u32 {
self.0 + rhs
}
}

impl Deref for _Equatable {
impl Deref for Equatable {
type Target = u32;

//~ MONO_ITEM fn <_Equatable as std::ops::Deref>::deref
//~ MONO_ITEM fn <Equatable as std::ops::Deref>::deref
fn deref(&self) -> &Self::Target {
&self.0
}
Expand Down
1 change: 0 additions & 1 deletion tests/ui/coherence/re-rebalance-coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
extern crate re_rebalance_coherence_lib as lib;
use lib::*;

#[allow(dead_code)]
struct Oracle;
impl Backend for Oracle {}
impl<'a, T:'a, Tab> QueryFragment<Oracle> for BatchInsert<'a, T, Tab> {}
Expand Down
1 change: 0 additions & 1 deletion tests/ui/const-generics/defaults/repr-c-issue-82792.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

//@ run-pass

#[allow(dead_code)]
#[repr(C)]
pub struct Loaf<T: Sized, const N: usize = 1> {
head: [T; N],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ impl BlockCipher for BarCipher {
const BLOCK_SIZE: usize = 32;
}

#[allow(dead_code)]
pub struct Block<C>(C);
pub struct Block<C>(#[allow(dead_code)] C);

pub fn test<C: BlockCipher, const M: usize>()
where
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

use std::mem::MaybeUninit;

#[allow(dead_code)]
#[repr(transparent)]
pub struct MaybeUninitWrapper<const N: usize>(MaybeUninit<[u64; N]>);

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/derives/clone-debug-dead-code-in-the-same-struct.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#![forbid(dead_code)]

#[derive(Debug)]
pub struct Whatever { //~ ERROR struct `Whatever` is never constructed
pub struct Whatever {
pub field0: (),
field1: (),
field1: (), //~ ERROR fields `field1`, `field2`, `field3`, and `field4` are never read
field2: (),
field3: (),
field4: (),
Expand Down
16 changes: 13 additions & 3 deletions tests/ui/derives/clone-debug-dead-code-in-the-same-struct.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
error: struct `Whatever` is never constructed
--> $DIR/clone-debug-dead-code-in-the-same-struct.rs:4:12
error: fields `field1`, `field2`, `field3`, and `field4` are never read
--> $DIR/clone-debug-dead-code-in-the-same-struct.rs:6:5
|
LL | pub struct Whatever {
| ^^^^^^^^
| -------- fields in this struct
LL | pub field0: (),
LL | field1: (),
| ^^^^^^
LL | field2: (),
| ^^^^^^
LL | field3: (),
| ^^^^^^
LL | field4: (),
| ^^^^^^
|
= note: `Whatever` has a derived impl for the trait `Debug`, but this is intentionally ignored during dead code analysis
note: the lint level is defined here
--> $DIR/clone-debug-dead-code-in-the-same-struct.rs:1:11
|
Expand Down
1 change: 0 additions & 1 deletion tests/ui/issues/issue-5708.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub trait MyTrait<T> {
fn dummy(&self, t: T) -> T { panic!() }
}

#[allow(dead_code)]
pub struct MyContainer<'a, T:'a> {
foos: Vec<&'a (dyn MyTrait<T>+'a)> ,
}
Expand Down
33 changes: 0 additions & 33 deletions tests/ui/lint/dead-code/allow-unconstructed-pub-struct.rs

This file was deleted.

5 changes: 3 additions & 2 deletions tests/ui/lint/dead-code/lint-dead-code-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,11 @@ struct SemiUsedStruct;
impl SemiUsedStruct {
fn la_la_la() {}
}
struct StructUsedAsField; //~ ERROR struct `StructUsedAsField` is never constructed
struct StructUsedAsField;
pub struct StructUsedInEnum;
struct StructUsedInGeneric;
pub struct PubStruct2 { //~ ERROR struct `PubStruct2` is never constructed
pub struct PubStruct2 {
#[allow(dead_code)]
struct_used_as_field: *const StructUsedAsField
}

Expand Down
Loading
Loading