From 02a42ff83d17ccb76ddf6dbaf1a0a56d91edd69c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 19 Jun 2022 18:52:06 +0200 Subject: [PATCH] Rewrite dead-code pass to avoid fetching HIR. --- compiler/rustc_passes/src/dead.rs | 265 ++++++------------ src/test/ui/lint/dead-code/issue-85255.stderr | 40 +-- .../ui/lint/dead-code/lint-dead-code-3.rs | 11 + .../ui/lint/dead-code/lint-dead-code-3.stderr | 26 +- .../ui/lint/dead-code/lint-dead-code-4.stderr | 6 +- 5 files changed, 146 insertions(+), 202 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 32c8bea0e2e..58c5e5b4dfe 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -10,14 +10,12 @@ use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{Node, PatKind, TyKind}; -use rustc_middle::hir::nested_filter; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::privacy; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, DefIdTree, TyCtxt}; use rustc_session::lint; use rustc_span::symbol::{sym, Symbol}; -use rustc_span::Span; use std::mem; // Any local node that may call something in its body block should be @@ -647,41 +645,16 @@ struct DeadVisitor<'tcx> { } impl<'tcx> DeadVisitor<'tcx> { - fn should_warn_about_item(&mut self, item: &hir::Item<'_>) -> bool { - let should_warn = matches!( - item.kind, - hir::ItemKind::Static(..) - | hir::ItemKind::Const(..) - | hir::ItemKind::Fn(..) - | hir::ItemKind::TyAlias(..) - | hir::ItemKind::Enum(..) - | hir::ItemKind::Struct(..) - | hir::ItemKind::Union(..) - ); - should_warn && !self.symbol_is_live(item.def_id) - } - - fn should_warn_about_field(&mut self, field: &hir::FieldDef<'_>) -> bool { - let def_id = self.tcx.hir().local_def_id(field.hir_id); - let field_type = self.tcx.type_of(def_id); - !field.is_positional() - && !self.symbol_is_live(def_id) - && !field_type.is_phantom_data() - && !has_allow_dead_code_or_lang_attr(self.tcx, field.hir_id) - } - - fn should_warn_about_variant(&mut self, variant: &hir::Variant<'_>) -> bool { - let def_id = self.tcx.hir().local_def_id(variant.id); - !self.symbol_is_live(def_id) && !has_allow_dead_code_or_lang_attr(self.tcx, variant.id) - } - - fn should_warn_about_foreign_item(&mut self, fi: &hir::ForeignItem<'_>) -> bool { - !self.symbol_is_live(fi.def_id) && !has_allow_dead_code_or_lang_attr(self.tcx, fi.hir_id()) - } - - // id := HIR id of an item's definition. - fn symbol_is_live(&mut self, def_id: LocalDefId) -> bool { - self.live_symbols.contains(&def_id) + fn should_warn_about_field(&mut self, field: &ty::FieldDef) -> bool { + if self.live_symbols.contains(&field.did.expect_local()) { + return false; + } + let is_positional = field.name.as_str().starts_with(|c: char| c.is_ascii_digit()); + if is_positional { + return false; + } + let field_type = self.tcx.type_of(field.did); + !field_type.is_phantom_data() } fn warn_multiple_dead_codes( @@ -790,143 +763,36 @@ impl<'tcx> DeadVisitor<'tcx> { } fn warn_dead_code(&mut self, id: LocalDefId, participle: &str) { - if self.tcx.item_name(id.to_def_id()).as_str().starts_with('_') { - return; - } self.warn_multiple_dead_codes(&[id], participle, None); } -} -impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> { - type NestedFilter = nested_filter::All; - - /// Walk nested items in place so that we don't report dead-code - /// on inner functions when the outer function is already getting - /// an error. We could do this also by checking the parents, but - /// this is how the code is setup and it seems harmless enough. - fn nested_visit_map(&mut self) -> Self::Map { - self.tcx.hir() - } - - fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { - if self.should_warn_about_item(item) { - // For most items, we want to highlight its identifier - let participle = match item.kind { - hir::ItemKind::Struct(..) => "constructed", // Issue #52325 - _ => "used", - }; - self.warn_dead_code(item.def_id, participle); - } else { - // Only continue if we didn't warn - intravisit::walk_item(self, item); + fn check_definition(&mut self, def_id: LocalDefId) { + if self.live_symbols.contains(&def_id) { + return; } - } - - // This visitor should only visit a single module at a time. - fn visit_mod(&mut self, _: &'tcx hir::Mod<'tcx>, _: Span, _: hir::HirId) {} - - fn visit_enum_def( - &mut self, - enum_definition: &'tcx hir::EnumDef<'tcx>, - generics: &'tcx hir::Generics<'tcx>, - item_id: hir::HirId, - _: Span, - ) { - intravisit::walk_enum_def(self, enum_definition, generics, item_id); - let dead_variants = enum_definition - .variants - .iter() - .filter_map(|variant| { - if self.should_warn_about_variant(&variant) { - Some(DeadVariant { - def_id: self.tcx.hir().local_def_id(variant.id), - name: variant.ident.name, - level: self.tcx.lint_level_at_node(lint::builtin::DEAD_CODE, variant.id).0, - }) - } else { - None - } - }) - .collect(); - self.warn_dead_fields_and_variants(item_id.expect_owner(), "constructed", dead_variants) - } - - fn visit_variant( - &mut self, - variant: &'tcx hir::Variant<'tcx>, - g: &'tcx hir::Generics<'tcx>, - id: hir::HirId, - ) { - if !self.should_warn_about_variant(&variant) { - intravisit::walk_variant(self, variant, g, id); + let hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id); + if has_allow_dead_code_or_lang_attr(self.tcx, hir_id) { + return; } - } - - fn visit_foreign_item(&mut self, fi: &'tcx hir::ForeignItem<'tcx>) { - if self.should_warn_about_foreign_item(fi) { - self.warn_dead_code(fi.def_id, "used"); + let Some(name) = self.tcx.opt_item_name(def_id.to_def_id()) else { + return + }; + if name.as_str().starts_with('_') { + return; } - intravisit::walk_foreign_item(self, fi); - } - - fn visit_variant_data( - &mut self, - def: &'tcx hir::VariantData<'tcx>, - _: Symbol, - _: &hir::Generics<'_>, - id: hir::HirId, - _: rustc_span::Span, - ) { - intravisit::walk_struct_def(self, def); - let dead_fields = def - .fields() - .iter() - .filter_map(|field| { - if self.should_warn_about_field(&field) { - Some(DeadVariant { - def_id: self.tcx.hir().local_def_id(field.hir_id), - name: field.ident.name, - level: self - .tcx - .lint_level_at_node(lint::builtin::DEAD_CODE, field.hir_id) - .0, - }) - } else { - None - } - }) - .collect(); - self.warn_dead_fields_and_variants(self.tcx.hir().local_def_id(id), "read", dead_fields) - } - - fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem<'tcx>) { - match impl_item.kind { - hir::ImplItemKind::Const(_, body_id) => { - if !self.symbol_is_live(impl_item.def_id) { - self.warn_dead_code(impl_item.def_id, "used"); - } - self.visit_nested_body(body_id) - } - hir::ImplItemKind::Fn(_, body_id) => { - if !self.symbol_is_live(impl_item.def_id) { - self.warn_dead_code(impl_item.def_id, "used"); - } - self.visit_nested_body(body_id) - } - hir::ImplItemKind::TyAlias(..) => {} - } - } - - // Overwrite so that we don't warn the trait item itself. - fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem<'tcx>) { - match trait_item.kind { - hir::TraitItemKind::Const(_, Some(body_id)) - | hir::TraitItemKind::Fn(_, hir::TraitFn::Provided(body_id)) => { - self.visit_nested_body(body_id) - } - hir::TraitItemKind::Const(_, None) - | hir::TraitItemKind::Fn(_, hir::TraitFn::Required(_)) - | hir::TraitItemKind::Type(..) => {} + match self.tcx.def_kind(def_id) { + DefKind::AssocConst + | DefKind::AssocFn + | DefKind::Fn + | DefKind::Static(_) + | DefKind::Const + | DefKind::TyAlias + | DefKind::Enum + | DefKind::Union + | DefKind::ForeignTy => 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"), + _ => {} } } } @@ -934,10 +800,65 @@ impl<'tcx> Visitor<'tcx> for DeadVisitor<'tcx> { fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalDefId) { let (live_symbols, ignored_derived_traits) = tcx.live_symbols_and_ignored_derived_traits(()); let mut visitor = DeadVisitor { tcx, live_symbols, ignored_derived_traits }; - let (module, _, module_id) = tcx.hir().get_module(module); - // Do not use an ItemLikeVisitor since we may want to skip visiting some items - // when a surrounding one is warned against or `_`. - intravisit::walk_mod(&mut visitor, module, module_id); + + let module_items = tcx.hir_module_items(module); + + for item in module_items.items() { + if !live_symbols.contains(&item.def_id) { + let parent = tcx.local_parent(item.def_id); + if parent != module && !live_symbols.contains(&parent) { + // We already have diagnosed something. + continue; + } + visitor.check_definition(item.def_id); + continue; + } + + let def_kind = tcx.def_kind(item.def_id); + if let DefKind::Struct | DefKind::Union | DefKind::Enum = def_kind { + let adt = tcx.adt_def(item.def_id); + let mut dead_variants = Vec::new(); + + for variant in adt.variants() { + let def_id = variant.def_id.expect_local(); + if !live_symbols.contains(&def_id) { + // Record to group diagnostics. + let hir_id = tcx.hir().local_def_id_to_hir_id(def_id); + let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0; + dead_variants.push(DeadVariant { def_id, name: variant.name, level }); + continue; + } + + let dead_fields = variant + .fields + .iter() + .filter_map(|field| { + let def_id = field.did.expect_local(); + let hir_id = tcx.hir().local_def_id_to_hir_id(def_id); + if visitor.should_warn_about_field(&field) { + let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0; + Some(DeadVariant { def_id, name: field.name, level }) + } else { + None + } + }) + .collect(); + visitor.warn_dead_fields_and_variants(def_id, "read", dead_fields) + } + + visitor.warn_dead_fields_and_variants(item.def_id, "constructed", dead_variants); + } + } + + for impl_item in module_items.impl_items() { + visitor.check_definition(impl_item.def_id); + } + + for foreign_item in module_items.foreign_items() { + visitor.check_definition(foreign_item.def_id); + } + + // We do not warn trait items. } pub(crate) fn provide(providers: &mut Providers) { diff --git a/src/test/ui/lint/dead-code/issue-85255.stderr b/src/test/ui/lint/dead-code/issue-85255.stderr index cff572e1b55..3497b952fdd 100644 --- a/src/test/ui/lint/dead-code/issue-85255.stderr +++ b/src/test/ui/lint/dead-code/issue-85255.stderr @@ -14,6 +14,26 @@ note: the lint level is defined here LL | #![warn(dead_code)] | ^^^^^^^^^ +warning: fields `a` and `b` are never read + --> $DIR/issue-85255.rs:19:5 + | +LL | pub(crate) struct Foo1 { + | ---- fields in this struct +LL | a: i32, + | ^ +LL | pub b: i32, + | ^ + +warning: fields `a` and `b` are never read + --> $DIR/issue-85255.rs:31:5 + | +LL | pub(crate) struct Foo2 { + | ---- fields in this struct +LL | a: i32, + | ^ +LL | pub b: i32, + | ^ + warning: associated function `a` is never used --> $DIR/issue-85255.rs:14:8 | @@ -26,16 +46,6 @@ warning: associated function `b` is never used LL | pub fn b(&self) -> i32 { 6 } | ^ -warning: fields `a` and `b` are never read - --> $DIR/issue-85255.rs:19:5 - | -LL | pub(crate) struct Foo1 { - | ---- fields in this struct -LL | a: i32, - | ^ -LL | pub b: i32, - | ^ - warning: associated function `a` is never used --> $DIR/issue-85255.rs:26:8 | @@ -48,16 +58,6 @@ warning: associated function `b` is never used LL | pub fn b(&self) -> i32 { 6 } | ^ -warning: fields `a` and `b` are never read - --> $DIR/issue-85255.rs:31:5 - | -LL | pub(crate) struct Foo2 { - | ---- fields in this struct -LL | a: i32, - | ^ -LL | pub b: i32, - | ^ - warning: associated function `a` is never used --> $DIR/issue-85255.rs:38:8 | diff --git a/src/test/ui/lint/dead-code/lint-dead-code-3.rs b/src/test/ui/lint/dead-code/lint-dead-code-3.rs index c3e56063dc3..293fcdbc5ee 100644 --- a/src/test/ui/lint/dead-code/lint-dead-code-3.rs +++ b/src/test/ui/lint/dead-code/lint-dead-code-3.rs @@ -73,7 +73,18 @@ mod inner { fn f() {} } +fn anon_const() -> [(); { + fn blah() {} //~ ERROR: function `blah` is never used + 1 +}] { + [(); { + fn blah() {} //~ ERROR: function `blah` is never used + 1 + }] +} + pub fn foo() { let a: &dyn inner::Trait = &1_isize; a.f(); + anon_const(); } diff --git a/src/test/ui/lint/dead-code/lint-dead-code-3.stderr b/src/test/ui/lint/dead-code/lint-dead-code-3.stderr index 38578929848..26fc13bae08 100644 --- a/src/test/ui/lint/dead-code/lint-dead-code-3.stderr +++ b/src/test/ui/lint/dead-code/lint-dead-code-3.stderr @@ -10,12 +10,6 @@ note: the lint level is defined here LL | #![deny(dead_code)] | ^^^^^^^^^ -error: associated function `foo` is never used - --> $DIR/lint-dead-code-3.rs:16:8 - | -LL | fn foo(&self) { - | ^^^ - error: function `bar` is never used --> $DIR/lint-dead-code-3.rs:21:4 | @@ -28,11 +22,29 @@ error: enum `c_void` is never used LL | enum c_void {} | ^^^^^^ +error: function `blah` is never used + --> $DIR/lint-dead-code-3.rs:77:8 + | +LL | fn blah() {} + | ^^^^ + +error: function `blah` is never used + --> $DIR/lint-dead-code-3.rs:81:12 + | +LL | fn blah() {} + | ^^^^ + +error: associated function `foo` is never used + --> $DIR/lint-dead-code-3.rs:16:8 + | +LL | fn foo(&self) { + | ^^^ + error: function `free` is never used --> $DIR/lint-dead-code-3.rs:62:8 | LL | fn free(p: *const c_void); | ^^^^ -error: aborting due to 5 previous errors +error: aborting due to 7 previous errors diff --git a/src/test/ui/lint/dead-code/lint-dead-code-4.stderr b/src/test/ui/lint/dead-code/lint-dead-code-4.stderr index 8cb4621d543..668c1dacf95 100644 --- a/src/test/ui/lint/dead-code/lint-dead-code-4.stderr +++ b/src/test/ui/lint/dead-code/lint-dead-code-4.stderr @@ -32,9 +32,9 @@ LL | enum ABC { error: fields `b` and `c` are never read --> $DIR/lint-dead-code-4.rs:39:9 | -LL | enum IJK { - | --- fields in this enum -... +LL | J { + | - fields in this variant +LL | a: String, LL | b: i32, | ^ LL | c: i32,