Skip to content

Commit

Permalink
Auto merge of #114417 - chinedufn:fix-expect-unused-in-impl-block-rus…
Browse files Browse the repository at this point in the history
…t-issue-114416, r=cjgillot

Fix multiple `expect` attribs in impl block

Closes #114416
  • Loading branch information
bors committed Oct 4, 2023
2 parents eea2614 + bf4df06 commit 2bbb619
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 34 deletions.
73 changes: 39 additions & 34 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// This implements the dead-code warning pass. It follows middle::reachable
// closely. The idea is that all reachable symbols are live, codes called
// from live codes are live, and everything else is dead.
// This implements the dead-code warning pass.
// All reachable symbols are live, code called from live code is live, code with certain lint
// expectations such as `#[expect(unused)]` and `#[expect(dead_code)]` is live, and everything else
// is dead.

use hir::def_id::{LocalDefIdMap, LocalDefIdSet};
use itertools::Itertools;
Expand Down Expand Up @@ -747,7 +748,7 @@ fn live_symbols_and_ignored_derived_traits(
(symbol_visitor.live_symbols, symbol_visitor.ignored_derived_traits)
}

struct DeadVariant {
struct DeadItem {
def_id: LocalDefId,
name: Symbol,
level: lint::Level,
Expand Down Expand Up @@ -785,7 +786,13 @@ impl<'tcx> DeadVisitor<'tcx> {
ShouldWarnAboutField::Yes(is_positional)
}

fn warn_multiple_dead_codes(
// # Panics
// All `dead_codes` must have the same lint level, otherwise we will intentionally ICE.
// This is because we emit a multi-spanned lint using the lint level of the `dead_codes`'s
// first local def id.
// Prefer calling `Self.warn_dead_code` or `Self.warn_dead_code_grouped_by_lint_level`
// since those methods group by lint level before calling this method.
fn lint_at_single_level(
&self,
dead_codes: &[LocalDefId],
participle: &str,
Expand All @@ -796,6 +803,15 @@ impl<'tcx> DeadVisitor<'tcx> {
return;
};
let tcx = self.tcx;

let first_hir_id = tcx.hir().local_def_id_to_hir_id(first_id);
let first_lint_level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, first_hir_id).0;
assert!(dead_codes.iter().skip(1).all(|id| {
let hir_id = tcx.hir().local_def_id_to_hir_id(*id);
let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0;
level == first_lint_level
}));

let names: Vec<_> =
dead_codes.iter().map(|&def_id| tcx.item_name(def_id.to_def_id())).collect();
let spans: Vec<_> = dead_codes
Expand Down Expand Up @@ -876,31 +892,26 @@ impl<'tcx> DeadVisitor<'tcx> {
}
};

self.tcx.emit_spanned_lint(
lint,
tcx.hir().local_def_id_to_hir_id(first_id),
MultiSpan::from_spans(spans),
diag,
);
self.tcx.emit_spanned_lint(lint, first_hir_id, MultiSpan::from_spans(spans), diag);
}

fn warn_dead_fields_and_variants(
fn warn_multiple(
&self,
def_id: LocalDefId,
participle: &str,
dead_codes: Vec<DeadVariant>,
dead_codes: Vec<DeadItem>,
is_positional: bool,
) {
let mut dead_codes = dead_codes
.iter()
.filter(|v| !v.name.as_str().starts_with('_'))
.collect::<Vec<&DeadVariant>>();
.collect::<Vec<&DeadItem>>();
if dead_codes.is_empty() {
return;
}
dead_codes.sort_by_key(|v| v.level);
for (_, group) in &dead_codes.into_iter().group_by(|v| v.level) {
self.warn_multiple_dead_codes(
self.lint_at_single_level(
&group.map(|v| v.def_id).collect::<Vec<_>>(),
participle,
Some(def_id),
Expand All @@ -910,7 +921,7 @@ impl<'tcx> DeadVisitor<'tcx> {
}

fn warn_dead_code(&mut self, id: LocalDefId, participle: &str) {
self.warn_multiple_dead_codes(&[id], participle, None, false);
self.lint_at_single_level(&[id], participle, None, false);
}

fn check_definition(&mut self, def_id: LocalDefId) {
Expand Down Expand Up @@ -954,17 +965,16 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
if let hir::ItemKind::Impl(impl_item) = tcx.hir().item(item).kind {
let mut dead_items = Vec::new();
for item in impl_item.items {
let did = item.id.owner_id.def_id;
if !visitor.is_live_code(did) {
dead_items.push(did)
let def_id = item.id.owner_id.def_id;
if !visitor.is_live_code(def_id) {
let name = tcx.item_name(def_id.to_def_id());
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_items.push(DeadItem { def_id, name, level })
}
}
visitor.warn_multiple_dead_codes(
&dead_items,
"used",
Some(item.owner_id.def_id),
false,
);
visitor.warn_multiple(item.owner_id.def_id, "used", dead_items, false);
}

if !live_symbols.contains(&item.owner_id.def_id) {
Expand All @@ -988,7 +998,7 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
// 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 });
dead_variants.push(DeadItem { def_id, name: variant.name, level });
continue;
}

Expand All @@ -1013,21 +1023,16 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalModDefId) {
hir_id,
)
.0;
Some(DeadVariant { def_id, name: field.name, level })
Some(DeadItem { def_id, name: field.name, level })
} else {
None
}
})
.collect();
visitor.warn_dead_fields_and_variants(def_id, "read", dead_fields, is_positional)
visitor.warn_multiple(def_id, "read", dead_fields, is_positional);
}

visitor.warn_dead_fields_and_variants(
item.owner_id.def_id,
"constructed",
dead_variants,
false,
);
visitor.warn_multiple(item.owner_id.def_id, "constructed", dead_variants, false);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// check-pass
// incremental

#![feature(lint_reasons)]
#![warn(unused)]

struct OneUnused;
struct TwoUnused;

impl OneUnused {
#[expect(unused)]
fn unused() {}
}

impl TwoUnused {
#[expect(unused)]
fn unused1(){}

// This unused method has `#[expect(unused)]`, so the compiler should not emit a warning.
// This ui test was added after a regression in the compiler where it did not recognize multiple
// `#[expect(unused)]` annotations inside of impl blocks.
// issue 114416
#[expect(unused)]
fn unused2(){}
}

fn main() {
let _ = OneUnused;
let _ = TwoUnused;
}

0 comments on commit 2bbb619

Please sign in to comment.