From 7db9c3c1a6ef12e3402c1cf439a203faeab1ca2c Mon Sep 17 00:00:00 2001 From: Chinedu Francis Nwafili Date: Tue, 12 Sep 2023 03:11:11 -0400 Subject: [PATCH] Tests passing --- compiler/rustc_passes/src/dead.rs | 34 +++++++++++-------- ...-114416-expect-unused-inside-impl-block.rs | 32 +++++++++++++++++ .../expect_unused_inside_impl_block.rs | 8 +++-- 3 files changed, 58 insertions(+), 16 deletions(-) create mode 100644 tests/incremental/issue-114416-expect-unused-inside-impl-block.rs diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 486bb3f2e15..3af34840af5 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -723,6 +723,12 @@ fn should_warn_about_field(&mut self, field: &ty::FieldDef) -> ShouldWarnAboutFi ShouldWarnAboutField::Yes(is_positional) } + // # 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 warn_multiple_dead_codes( &self, dead_codes: &[LocalDefId], @@ -734,6 +740,15 @@ fn warn_multiple_dead_codes( 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 @@ -814,18 +829,9 @@ fn warn_multiple_dead_codes( } }; - // FIXME: Remove this before landing the PR. - // Just keeping it around so that I remember how to get the expectation id. - // for id in &dead_codes[1..] { - // let hir = self.tcx.hir().local_def_id_to_hir_id(*id); - // let lint_level = self.tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir).0; - // if let Some(expectation_id) = lint_level.get_expectation_id() { - // self.tcx.sess.diagnostic().insert_fulfilled_expectation(expectation_id); - // } - // } self.tcx.emit_spanned_lint( lint, - tcx.hir().local_def_id_to_hir_id(first_id), + first_hir_id, MultiSpan::from_spans(spans), diag, ); @@ -903,14 +909,14 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalDefId) { 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) { + 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 = tcx.hir().local_def_id_to_hir_id(did); + let hir = tcx.hir().local_def_id_to_hir_id(def_id); let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir).0; dead_items.push(DeadVariant { - def_id: did, + def_id, name, level, }) diff --git a/tests/incremental/issue-114416-expect-unused-inside-impl-block.rs b/tests/incremental/issue-114416-expect-unused-inside-impl-block.rs new file mode 100644 index 00000000000..309fa2e97ec --- /dev/null +++ b/tests/incremental/issue-114416-expect-unused-inside-impl-block.rs @@ -0,0 +1,32 @@ +// revisions: rpass1 +// +// The corresponding ui test can be found in +// `tests/ui/lint/rfc-2383-lint-reason/expect_unused_inside_impl_block.rs` + +#![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; +} diff --git a/tests/ui/lint/rfc-2383-lint-reason/expect_unused_inside_impl_block.rs b/tests/ui/lint/rfc-2383-lint-reason/expect_unused_inside_impl_block.rs index ee6aacb290b..425f488680e 100644 --- a/tests/ui/lint/rfc-2383-lint-reason/expect_unused_inside_impl_block.rs +++ b/tests/ui/lint/rfc-2383-lint-reason/expect_unused_inside_impl_block.rs @@ -1,4 +1,7 @@ // check-pass +// +// The corresponding incremental compilation test can be found in +// `tests/incremental/issue-114416-expect-unused-inside-impl-block.rs` #![feature(lint_reasons)] #![warn(unused)] @@ -15,8 +18,9 @@ impl TwoUnused { #[expect(unused)] fn unused1(){} - // Tests a regression where the compiler erroneously determined that all `#[expect(unused)]` - // after the first method in the impl block were unfulfilled. + // 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(){}