mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Respect #[expect] the same way #[allow] is with the dead_code lint

Browse Source
This commit is contained in:
Urgau 2023-08-10 19:23:17 +02:00
parent c435af0d5c
commit d801a2ff15
6 changed files with 163 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

126
compiler/rustc_passes/src/dead.rs
View File

@ -4,6 +4,7 @@
use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
use itertools::Itertools;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::MultiSpan;
use rustc_hir as hir;
use rustc_hir::def::{CtorOf, DefKind, Res};
@ -42,8 +43,16 @@ fn should_explore(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
)
}
/// Determine if a work from the worklist is coming from the a `#[allow]`
/// or a `#[expect]` of `dead_code`
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
enum ComesFromAllowExpect {
Yes,
No,
}
struct MarkSymbolVisitor<'tcx> {
worklist: Vec<LocalDefId>,
worklist: Vec<(LocalDefId, ComesFromAllowExpect)>,
tcx: TyCtxt<'tcx>,
maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>,
live_symbols: LocalDefIdSet,
@ -72,7 +81,7 @@ fn typeck_results(&self) -> &'tcx ty::TypeckResults<'tcx> {
fn check_def_id(&mut self, def_id: DefId) {
if let Some(def_id) = def_id.as_local() {
if should_explore(self.tcx, def_id) || self.struct_constructors.contains_key(&def_id) {
self.worklist.push(def_id);
self.worklist.push((def_id, ComesFromAllowExpect::No));
}
self.live_symbols.insert(def_id);
}
@ -269,12 +278,14 @@ fn handle_offset_of(&mut self, expr: &'tcx hir::Expr<'tcx>) {
}
fn mark_live_symbols(&mut self) {
let mut scanned = LocalDefIdSet::default();
while let Some(id) = self.worklist.pop() {
if !scanned.insert(id) {
let mut scanned = UnordSet::default();
while let Some(work) = self.worklist.pop() {
if !scanned.insert(work) {
continue;
}
let (id, comes_from_allow_expect) = work;
// Avoid accessing the HIR for the synthesized associated type generated for RPITITs.
if self.tcx.is_impl_trait_in_trait(id.to_def_id()) {
self.live_symbols.insert(id);
@ -286,7 +297,30 @@ fn mark_live_symbols(&mut self) {
let id = self.struct_constructors.get(&id).copied().unwrap_or(id);
if let Some(node) = self.tcx.hir().find_by_def_id(id) {
self.live_symbols.insert(id);
// When using `#[allow]` or `#[expect]` of `dead_code`, we do a QOL improvement
// by declaring fn calls, statics, ... within said items as live, as well as
// the item itself, although technically this is not the case.
//
// This means that the lint for said items will never be fired.
//
// This doesn't make any difference for the item declared with `#[allow]`, as
// the lint firing will be a nop, as it will be silenced by the `#[allow]` of
// the item.
//
// However, for `#[expect]`, the presence or absence of the lint is relevant,
// so we don't add it to the list of live symbols when it comes from a
// `#[expect]`. This means that we will correctly report an item as live or not
// for the `#[expect]` case.
//
// Note that an item can and will be duplicated on the worklist with different
// `ComesFromAllowExpect`, particulary if it was added from the
// `effective_visibilities` query or from the `#[allow]`/`#[expect]` checks,
// this "duplication" is essential as otherwise a function with `#[expect]`
// called from a `pub fn` may be falsely reported as not live, falsely
// triggering the `unfulfilled_lint_expectations` lint.
if comes_from_allow_expect != ComesFromAllowExpect::Yes {
self.live_symbols.insert(id);
}
self.visit_node(node);
}
}
@ -513,16 +547,20 @@ fn visit_inline_const(&mut self, c: &'tcx hir::ConstBlock) {
}
}
fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
fn has_allow_dead_code_or_lang_attr(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
) -> Option<ComesFromAllowExpect> {
fn has_lang_attr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
tcx.has_attr(def_id, sym::lang)
// Stable attribute for #[lang = "panic_impl"]
|| tcx.has_attr(def_id, sym::panic_handler)
}
fn has_allow_dead_code(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
fn has_allow_expect_dead_code(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0 == lint::Allow
let lint_level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0;
matches!(lint_level, lint::Allow | lint::Expect(_))
}
fn has_used_like_attr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
@ -537,9 +575,13 @@ fn has_used_like_attr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
}
}
has_allow_dead_code(tcx, def_id)
|| has_used_like_attr(tcx, def_id)
|| has_lang_attr(tcx, def_id)
if has_allow_expect_dead_code(tcx, def_id) {
Some(ComesFromAllowExpect::Yes)
} else if has_used_like_attr(tcx, def_id) || has_lang_attr(tcx, def_id) {
Some(ComesFromAllowExpect::No)
} else {
None
}
}
// These check_* functions seeds items that
@ -557,21 +599,23 @@ fn has_used_like_attr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
// * Implementations of traits and trait methods
fn check_item<'tcx>(
tcx: TyCtxt<'tcx>,
worklist: &mut Vec<LocalDefId>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
struct_constructors: &mut LocalDefIdMap<LocalDefId>,
id: hir::ItemId,
) {
let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id);
if allow_dead_code {
worklist.push(id.owner_id.def_id);
if let Some(comes_from_allow) = allow_dead_code {
worklist.push((id.owner_id.def_id, comes_from_allow));
}
match tcx.def_kind(id.owner_id) {
DefKind::Enum => {
let item = tcx.hir().item(id);
if let hir::ItemKind::Enum(ref enum_def, _) = item.kind {
if allow_dead_code {
worklist.extend(enum_def.variants.iter().map(|variant| variant.def_id));
if let Some(comes_from_allow) = allow_dead_code {
worklist.extend(
enum_def.variants.iter().map(|variant| (variant.def_id, comes_from_allow)),
);
}
for variant in enum_def.variants {
@ -583,7 +627,7 @@ fn check_item<'tcx>(
}
DefKind::Impl { of_trait } => {
if of_trait {
worklist.push(id.owner_id.def_id);
worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No));
}
// get DefIds from another query
@ -594,8 +638,10 @@ fn check_item<'tcx>(
// And we access the Map here to get HirId from LocalDefId
for id in local_def_ids {
if of_trait || has_allow_dead_code_or_lang_attr(tcx, id) {
worklist.push(id);
if of_trait {
worklist.push((id, ComesFromAllowExpect::No));
} else if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, id) {
worklist.push((id, comes_from_allow));
}
}
}
@ -609,43 +655,59 @@ fn check_item<'tcx>(
}
DefKind::GlobalAsm => {
// global_asm! is always live.
worklist.push(id.owner_id.def_id);
worklist.push((id.owner_id.def_id, ComesFromAllowExpect::No));
}
_ => {}
}
}
fn check_trait_item(tcx: TyCtxt<'_>, worklist: &mut Vec<LocalDefId>, id: hir::TraitItemId) {
fn check_trait_item(
tcx: TyCtxt<'_>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
id: hir::TraitItemId,
) {
use hir::TraitItemKind::{Const, Fn};
if matches!(tcx.def_kind(id.owner_id), DefKind::AssocConst | DefKind::AssocFn) {
let trait_item = tcx.hir().trait_item(id);
if matches!(trait_item.kind, Const(_, Some(_)) | Fn(_, hir::TraitFn::Provided(_)))
&& has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id)
&& let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, trait_item.owner_id.def_id)
{
worklist.push(trait_item.owner_id.def_id);
worklist.push((trait_item.owner_id.def_id, comes_from_allow));
}
}
}
fn check_foreign_item(tcx: TyCtxt<'_>, worklist: &mut Vec<LocalDefId>, id: hir::ForeignItemId) {
fn check_foreign_item(
tcx: TyCtxt<'_>,
worklist: &mut Vec<(LocalDefId, ComesFromAllowExpect)>,
id: hir::ForeignItemId,
) {
if matches!(tcx.def_kind(id.owner_id), DefKind::Static(_) | DefKind::Fn)
&& has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id)
&& let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, id.owner_id.def_id)
{
worklist.push(id.owner_id.def_id);
worklist.push((id.owner_id.def_id, comes_from_allow));
}
}
fn create_and_seed_worklist(tcx: TyCtxt<'_>) -> (Vec<LocalDefId>, LocalDefIdMap<LocalDefId>) {
fn create_and_seed_worklist(
tcx: TyCtxt<'_>,
) -> (Vec<(LocalDefId, ComesFromAllowExpect)>, LocalDefIdMap<LocalDefId>) {
let effective_visibilities = &tcx.effective_visibilities(());
// see `MarkSymbolVisitor::struct_constructors`
let mut struct_constructors = Default::default();
let mut worklist = effective_visibilities
.iter()
.filter_map(|(&id, effective_vis)| {
effective_vis.is_public_at_level(Level::Reachable).then_some(id)
effective_vis
.is_public_at_level(Level::Reachable)
.then_some(id)
.map(|id| (id, ComesFromAllowExpect::No))
})
// Seed entry point
.chain(tcx.entry_fn(()).and_then(|(def_id, _)| def_id.as_local()))
.chain(
tcx.entry_fn(())
.and_then(|(def_id, _)| def_id.as_local().map(|id| (id, ComesFromAllowExpect::No))),
)
.collect::<Vec<_>>();
let crate_items = tcx.hir_crate_items(());
@ -878,9 +940,7 @@ fn is_live_code(&self, def_id: LocalDefId) -> bool {
return true;
};
self.live_symbols.contains(&def_id)
|| has_allow_dead_code_or_lang_attr(self.tcx, def_id)
|| name.as_str().starts_with('_')
self.live_symbols.contains(&def_id) || name.as_str().starts_with('_')
}
}

19
tests/ui/lint/dead-code/allow-or-expect-dead_code-114557-2.rs Normal file
View File

@ -0,0 +1,19 @@
// check-pass
// this test checks that the `dead_code` lint is *NOT* being emited
// for `foo` as `foo` is being used by `main`, and so the `#[expect]`
// is unfulfilled
//
// it also checks that the `dead_code` lint is also *NOT* emited
// for `bar` as it's suppresed by the `#[expect]` on `bar`
#![feature(lint_reasons)]
#![warn(dead_code)] // to override compiletest
fn bar() {}
#[expect(dead_code)]
//~^ WARN this lint expectation is unfulfilled
fn foo() { bar() }
fn main() { foo() }

10
tests/ui/lint/dead-code/allow-or-expect-dead_code-114557-2.stderr Normal file
View File

@ -0,0 +1,10 @@
warning: this lint expectation is unfulfilled
--> $DIR/allow-or-expect-dead_code-114557-2.rs:15:10
|
LL | #[expect(dead_code)]
| ^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
warning: 1 warning emitted

13
tests/ui/lint/dead-code/allow-or-expect-dead_code-114557-3.rs Normal file
View File

@ -0,0 +1,13 @@
// check-pass
// this test makes sure that the `unfulfilled_lint_expectations` lint
// is being emited for `foo` as foo is not dead code, it's pub
#![feature(lint_reasons)]
#![warn(dead_code)] // to override compiletest
#[expect(dead_code)]
//~^ WARN this lint expectation is unfulfilled
pub fn foo() {}
fn main() {}

10
tests/ui/lint/dead-code/allow-or-expect-dead_code-114557-3.stderr Normal file
View File

@ -0,0 +1,10 @@
warning: this lint expectation is unfulfilled
--> $DIR/allow-or-expect-dead_code-114557-3.rs:9:10
|
LL | #[expect(dead_code)]
| ^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
warning: 1 warning emitted

18
tests/ui/lint/dead-code/allow-or-expect-dead_code-114557.rs Normal file
View File

@ -0,0 +1,18 @@
// check-pass
// revisions: allow expect
// this test checks that no matter if we put #[allow(dead_code)]
// or #[expect(dead_code)], no warning is being emited
#![feature(lint_reasons)]
#![warn(dead_code)] // to override compiletest
fn f() {}
#[cfg_attr(allow, allow(dead_code))]
#[cfg_attr(expect, expect(dead_code))]
fn g() {
f();
}
fn main() {}