Skip to content

Commit

Permalink
Revert "Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enha…
Browse files Browse the repository at this point in the history
…nce, r=pnkfelix"

This reverts commit 13314df, reversing
changes made to 6e534c7.
  • Loading branch information
compiler-errors authored and pietroalbini committed Aug 4, 2024
1 parent d45312d commit 8406ccc
Show file tree
Hide file tree
Showing 32 changed files with 88 additions and 272 deletions.
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()),
_ => 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
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

0 comments on commit 8406ccc

Please sign in to comment.