Skip to content

Commit 9bbbf60

Browse files
borsbors
committed
Auto merge of rust-lang#95977 - FabianWolff:issue-92790-dead-tuple, r=estebank
Warn about dead tuple struct fields Continuation of rust-lang#92972. Fixes rust-lang#92790. The language team has already commented on this in rust-lang#92972 (comment); I have incorporated their requests here. Specifically, there is now a new allow-by-default `unused_tuple_struct_fields` lint (name bikesheddable), and fields of unit type are ignored (rust-lang#92972 (comment)), so error messages look like this: ``` error: field is never read: `1` --> $DIR/tuple-struct-field.rs:6:21 | LL | struct Wrapper(i32, [u8; LEN], String); | ^^^^^^^^^ | help: change the field to unit type to suppress this warning while preserving the field numbering | LL | struct Wrapper(i32, (), String); | ~~ ``` r? `@joshtriplett`
2 parents cdfd675 + e3c7e04 commit 9bbbf60

File tree

168 files changed

+451
-246
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

168 files changed

+451
-246
lines changed

‎compiler/rustc_apfloat/src/ppc.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub type DoubleDouble = DoubleFloat<ieee::Double>;
3030
// FIXME: Implement all operations in DoubleDouble, and delete these
3131
// semantics.
3232
// FIXME(eddyb) This shouldn't need to be `pub`, it's only used in bounds.
33-
pub struct FallbackS<F>(F);
33+
pub struct FallbackS<F>(#[allow(unused)] F);
3434
type Fallback<F> = ieee::IeeeFloat<FallbackS<F>>;
3535
impl<F: Float> ieee::Semantics for FallbackS<F> {
3636
// Forbid any conversion to/from bits.
@@ -45,7 +45,7 @@ impl<F: Float> ieee::Semantics for FallbackS<F> {
4545
// truncate the mantissa. The result of that second conversion
4646
// may be inexact, but should never underflow.
4747
// FIXME(eddyb) This shouldn't need to be `pub`, it's only used in bounds.
48-
pub struct FallbackExtendedS<F>(F);
48+
pub struct FallbackExtendedS<F>(#[allow(unused)] F);
4949
type FallbackExtended<F> = ieee::IeeeFloat<FallbackExtendedS<F>>;
5050
impl<F: Float> ieee::Semantics for FallbackExtendedS<F> {
5151
// Forbid any conversion to/from bits.

‎compiler/rustc_lint_defs/src/builtin.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,32 @@ declare_lint! {
630630
"detects attributes that were not used by the compiler"
631631
}
632632

633+
declare_lint! {
634+
/// The `unused_tuple_struct_fields` lint detects fields of tuple structs
635+
/// that are never read.
636+
///
637+
/// ### Example
638+
///
639+
/// ```
640+
/// #[warn(unused_tuple_struct_fields)]
641+
/// struct S(i32, i32, i32);
642+
/// let s = S(1, 2, 3);
643+
/// let _ = (s.0, s.2);
644+
/// ```
645+
///
646+
/// {{produces}}
647+
///
648+
/// ### Explanation
649+
///
650+
/// Tuple struct fields that are never read anywhere may indicate a
651+
/// mistake or unfinished code. To silence this warning, consider
652+
/// removing the unused field(s) or, to preserve the numbering of the
653+
/// remaining fields, change the unused field(s) to have unit type.
654+
pub UNUSED_TUPLE_STRUCT_FIELDS,
655+
Allow,
656+
"detects tuple struct fields that are never read"
657+
}
658+
633659
declare_lint! {
634660
/// The `unreachable_code` lint detects unreachable code paths.
635661
///
@@ -3281,6 +3307,7 @@ declare_lint_pass! {
32813307
UNSUPPORTED_CALLING_CONVENTIONS,
32823308
BREAK_WITH_LABEL_AND_LOOP,
32833309
UNUSED_ATTRIBUTES,
3310+
UNUSED_TUPLE_STRUCT_FIELDS,
32843311
NON_EXHAUSTIVE_OMITTED_PATTERNS,
32853312
TEXT_DIRECTION_CODEPOINT_IN_COMMENT,
32863313
DEREF_INTO_DYN_SUPERTRAIT,

‎compiler/rustc_passes/src/check_attr.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ pub(crate) fn target_from_impl_item<'tcx>(
5353
#[derive(Clone, Copy)]
5454
enum ItemLike<'tcx> {
5555
Item(&'tcx Item<'tcx>),
56-
ForeignItem(&'tcx ForeignItem<'tcx>),
56+
ForeignItem,
5757
}
5858

5959
struct CheckAttrVisitor<'tcx> {
@@ -2020,12 +2020,7 @@ impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
20202020

20212021
fn visit_foreign_item(&mut self, f_item: &'tcx ForeignItem<'tcx>) {
20222022
let target = Target::from_foreign_item(f_item);
2023-
self.check_attributes(
2024-
f_item.hir_id(),
2025-
f_item.span,
2026-
target,
2027-
Some(ItemLike::ForeignItem(f_item)),
2028-
);
2023+
self.check_attributes(f_item.hir_id(), f_item.span, target, Some(ItemLike::ForeignItem));
20292024
intravisit::walk_foreign_item(self, f_item)
20302025
}
20312026

‎compiler/rustc_passes/src/dead.rs‎

Lines changed: 111 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
use itertools::Itertools;
66
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
7-
use rustc_errors::{pluralize, MultiSpan};
7+
use rustc_errors::{pluralize, Applicability, MultiSpan};
88
use rustc_hir as hir;
99
use rustc_hir::def::{CtorOf, DefKind, Res};
1010
use rustc_hir::def_id::{DefId, LocalDefId};
@@ -42,6 +42,7 @@ struct MarkSymbolVisitor<'tcx> {
4242
maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>,
4343
live_symbols: FxHashSet<LocalDefId>,
4444
repr_has_repr_c: bool,
45+
repr_has_repr_simd: bool,
4546
in_pat: bool,
4647
ignore_variant_stack: Vec<DefId>,
4748
// maps from tuple struct constructors to tuple struct items
@@ -220,6 +221,32 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
220221
}
221222
}
222223

224+
fn handle_tuple_field_pattern_match(
225+
&mut self,
226+
lhs: &hir::Pat<'_>,
227+
res: Res,
228+
pats: &[hir::Pat<'_>],
229+
dotdot: Option<usize>,
230+
) {
231+
let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
232+
ty::Adt(adt, _) => adt.variant_of_res(res),
233+
_ => span_bug!(lhs.span, "non-ADT in tuple struct pattern"),
234+
};
235+
let first_n = pats.iter().enumerate().take(dotdot.unwrap_or(pats.len()));
236+
let missing = variant.fields.len() - pats.len();
237+
let last_n = pats
238+
.iter()
239+
.enumerate()
240+
.skip(dotdot.unwrap_or(pats.len()))
241+
.map(|(idx, pat)| (idx + missing, pat));
242+
for (idx, pat) in first_n.chain(last_n) {
243+
if let PatKind::Wild = pat.kind {
244+
continue;
245+
}
246+
self.insert_def_id(variant.fields[idx].did);
247+
}
248+
}
249+
223250
fn mark_live_symbols(&mut self) {
224251
let mut scanned = FxHashSet::default();
225252
while let Some(id) = self.worklist.pop() {
@@ -274,12 +301,15 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
274301
}
275302

276303
let had_repr_c = self.repr_has_repr_c;
304+
let had_repr_simd = self.repr_has_repr_simd;
277305
self.repr_has_repr_c = false;
306+
self.repr_has_repr_simd = false;
278307
match node {
279308
Node::Item(item) => match item.kind {
280309
hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
281310
let def = self.tcx.adt_def(item.def_id);
282311
self.repr_has_repr_c = def.repr().c();
312+
self.repr_has_repr_simd = def.repr().simd();
283313

284314
intravisit::walk_item(self, &item)
285315
}
@@ -315,6 +345,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
315345
}
316346
_ => {}
317347
}
348+
self.repr_has_repr_simd = had_repr_simd;
318349
self.repr_has_repr_c = had_repr_c;
319350
}
320351

@@ -347,9 +378,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
347378
) {
348379
let tcx = self.tcx;
349380
let has_repr_c = self.repr_has_repr_c;
381+
let has_repr_simd = self.repr_has_repr_simd;
350382
let live_fields = def.fields().iter().filter_map(|f| {
351383
let def_id = tcx.hir().local_def_id(f.hir_id);
352-
if has_repr_c {
384+
if has_repr_c || (f.is_positional() && has_repr_simd) {
353385
return Some(def_id);
354386
}
355387
if !tcx.visibility(f.hir_id.owner).is_public() {
@@ -408,6 +440,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
408440
let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
409441
self.handle_res(res);
410442
}
443+
PatKind::TupleStruct(ref qpath, ref fields, dotdot) => {
444+
let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
445+
self.handle_tuple_field_pattern_match(pat, res, fields, dotdot);
446+
}
411447
_ => (),
412448
}
413449

@@ -440,7 +476,11 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
440476
}
441477
}
442478

443-
fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
479+
fn has_allow_dead_code_or_lang_attr_helper(
480+
tcx: TyCtxt<'_>,
481+
id: hir::HirId,
482+
lint: &'static lint::Lint,
483+
) -> bool {
444484
let attrs = tcx.hir().attrs(id);
445485
if tcx.sess.contains_name(attrs, sym::lang) {
446486
return true;
@@ -470,7 +510,11 @@ fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
470510
}
471511
}
472512

473-
tcx.lint_level_at_node(lint::builtin::DEAD_CODE, id).0 == lint::Allow
513+
tcx.lint_level_at_node(lint, id).0 == lint::Allow
514+
}
515+
516+
fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
517+
has_allow_dead_code_or_lang_attr_helper(tcx, id, lint::builtin::DEAD_CODE)
474518
}
475519

476520
// These check_* functions seeds items that
@@ -623,6 +667,7 @@ fn live_symbols_and_ignored_derived_traits<'tcx>(
623667
maybe_typeck_results: None,
624668
live_symbols: Default::default(),
625669
repr_has_repr_c: false,
670+
repr_has_repr_simd: false,
626671
in_pat: false,
627672
ignore_variant_stack: vec![],
628673
struct_constructors,
@@ -644,32 +689,46 @@ struct DeadVisitor<'tcx> {
644689
ignored_derived_traits: &'tcx FxHashMap<LocalDefId, Vec<(DefId, DefId)>>,
645690
}
646691

692+
enum ShouldWarnAboutField {
693+
Yes(bool), // positional?
694+
No,
695+
}
696+
647697
impl<'tcx> DeadVisitor<'tcx> {
648-
fn should_warn_about_field(&mut self, field: &ty::FieldDef) -> bool {
698+
fn should_warn_about_field(&mut self, field: &ty::FieldDef) -> ShouldWarnAboutField {
649699
if self.live_symbols.contains(&field.did.expect_local()) {
650-
return false;
700+
return ShouldWarnAboutField::No;
701+
}
702+
let field_type = self.tcx.type_of(field.did);
703+
if field_type.is_phantom_data() {
704+
return ShouldWarnAboutField::No;
651705
}
652706
let is_positional = field.name.as_str().starts_with(|c: char| c.is_ascii_digit());
653-
if is_positional {
654-
return false;
707+
if is_positional
708+
&& self
709+
.tcx
710+
.layout_of(self.tcx.param_env(field.did).and(field_type))
711+
.map_or(true, |layout| layout.is_zst())
712+
{
713+
return ShouldWarnAboutField::No;
655714
}
656-
let field_type = self.tcx.type_of(field.did);
657-
!field_type.is_phantom_data()
715+
ShouldWarnAboutField::Yes(is_positional)
658716
}
659717

660718
fn warn_multiple_dead_codes(
661719
&self,
662720
dead_codes: &[LocalDefId],
663721
participle: &str,
664722
parent_item: Option<LocalDefId>,
723+
is_positional: bool,
665724
) {
666725
if let Some(&first_id) = dead_codes.first() {
667726
let tcx = self.tcx;
668727
let names: Vec<_> = dead_codes
669728
.iter()
670729
.map(|&def_id| tcx.item_name(def_id.to_def_id()).to_string())
671730
.collect();
672-
let spans = dead_codes
731+
let spans: Vec<_> = dead_codes
673732
.iter()
674733
.map(|&def_id| match tcx.def_ident_span(def_id) {
675734
Some(s) => s.with_ctxt(tcx.def_span(def_id).ctxt()),
@@ -678,9 +737,13 @@ impl<'tcx> DeadVisitor<'tcx> {
678737
.collect();
679738

680739
tcx.struct_span_lint_hir(
681-
lint::builtin::DEAD_CODE,
740+
if is_positional {
741+
lint::builtin::UNUSED_TUPLE_STRUCT_FIELDS
742+
} else {
743+
lint::builtin::DEAD_CODE
744+
},
682745
tcx.hir().local_def_id_to_hir_id(first_id),
683-
MultiSpan::from_spans(spans),
746+
MultiSpan::from_spans(spans.clone()),
684747
|lint| {
685748
let descr = tcx.def_kind(first_id).descr(first_id.to_def_id());
686749
let span_len = dead_codes.len();
@@ -702,6 +765,21 @@ impl<'tcx> DeadVisitor<'tcx> {
702765
are = pluralize!("is", span_len),
703766
));
704767

768+
if is_positional {
769+
err.multipart_suggestion(
770+
&format!(
771+
"consider changing the field{s} to be of unit type to \
772+
suppress this warning while preserving the field \
773+
numbering, or remove the field{s}",
774+
s = pluralize!(span_len)
775+
),
776+
spans.iter().map(|sp| (*sp, "()".to_string())).collect(),
777+
// "HasPlaceholders" because applying this fix by itself isn't
778+
// enough: All constructor calls have to be adjusted as well
779+
Applicability::HasPlaceholders,
780+
);
781+
}
782+
705783
if let Some(parent_item) = parent_item {
706784
let parent_descr = tcx.def_kind(parent_item).descr(parent_item.to_def_id());
707785
err.span_label(
@@ -743,6 +821,7 @@ impl<'tcx> DeadVisitor<'tcx> {
743821
def_id: LocalDefId,
744822
participle: &str,
745823
dead_codes: Vec<DeadVariant>,
824+
is_positional: bool,
746825
) {
747826
let mut dead_codes = dead_codes
748827
.iter()
@@ -758,12 +837,13 @@ impl<'tcx> DeadVisitor<'tcx> {
758837
&group.map(|v| v.def_id).collect::<Vec<_>>(),
759838
participle,
760839
Some(def_id),
840+
is_positional,
761841
);
762842
}
763843
}
764844

765845
fn warn_dead_code(&mut self, id: LocalDefId, participle: &str) {
766-
self.warn_multiple_dead_codes(&[id], participle, None);
846+
self.warn_multiple_dead_codes(&[id], participle, None, false);
767847
}
768848

769849
fn check_definition(&mut self, def_id: LocalDefId) {
@@ -829,24 +909,37 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalDefId) {
829909
continue;
830910
}
831911

912+
let mut is_positional = false;
832913
let dead_fields = variant
833914
.fields
834915
.iter()
835916
.filter_map(|field| {
836917
let def_id = field.did.expect_local();
837918
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
838-
if visitor.should_warn_about_field(&field) {
839-
let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0;
919+
if let ShouldWarnAboutField::Yes(is_pos) =
920+
visitor.should_warn_about_field(&field)
921+
{
922+
let level = tcx
923+
.lint_level_at_node(
924+
if is_pos {
925+
is_positional = true;
926+
lint::builtin::UNUSED_TUPLE_STRUCT_FIELDS
927+
} else {
928+
lint::builtin::DEAD_CODE
929+
},
930+
hir_id,
931+
)
932+
.0;
840933
Some(DeadVariant { def_id, name: field.name, level })
841934
} else {
842935
None
843936
}
844937
})
845938
.collect();
846-
visitor.warn_dead_fields_and_variants(def_id, "read", dead_fields)
939+
visitor.warn_dead_fields_and_variants(def_id, "read", dead_fields, is_positional)
847940
}
848941

849-
visitor.warn_dead_fields_and_variants(item.def_id, "constructed", dead_variants);
942+
visitor.warn_dead_fields_and_variants(item.def_id, "constructed", dead_variants, false);
850943
}
851944
}
852945

‎src/test/codegen-units/item-collection/generic-drop-glue.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ enum EnumNoDrop<T1, T2> {
3434
}
3535

3636

37-
struct NonGenericNoDrop(i32);
37+
struct NonGenericNoDrop(#[allow(unused_tuple_struct_fields)] i32);
3838

39-
struct NonGenericWithDrop(i32);
39+
struct NonGenericWithDrop(#[allow(unused_tuple_struct_fields)] i32);
4040
//~ MONO_ITEM fn std::ptr::drop_in_place::<NonGenericWithDrop> - shim(Some(NonGenericWithDrop)) @@ generic_drop_glue-cgu.0[Internal]
4141

4242
impl Drop for NonGenericWithDrop {

0 commit comments

Comments
 (0)