Warn about dead tuple struct fields · rust-lang/rust@d59b34b (original) (raw)
`@@ -4,7 +4,7 @@
`
4
4
``
5
5
`use itertools::Itertools;
`
6
6
`use rustc_data_structures::fx::{FxHashMap, FxHashSet};
`
7
``
`-
use rustc_errors::{pluralize, MultiSpan};
`
``
7
`+
use rustc_errors::{pluralize, Applicability, MultiSpan};
`
8
8
`use rustc_hir as hir;
`
9
9
`use rustc_hir::def::{CtorOf, DefKind, Res};
`
10
10
`use rustc_hir::def_id::{DefId, LocalDefId};
`
`@@ -42,6 +42,7 @@ struct MarkSymbolVisitor<'tcx> {
`
42
42
`maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>,
`
43
43
`live_symbols: FxHashSet,
`
44
44
`repr_has_repr_c: bool,
`
``
45
`+
repr_has_repr_simd: bool,
`
45
46
`in_pat: bool,
`
46
47
`ignore_variant_stack: Vec,
`
47
48
`// maps from tuple struct constructors to tuple struct items
`
`@@ -220,6 +221,32 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
`
220
221
`}
`
221
222
`}
`
222
223
``
``
224
`+
fn handle_tuple_field_pattern_match(
`
``
225
`+
&mut self,
`
``
226
`+
lhs: &hir::Pat<'_>,
`
``
227
`+
res: Res,
`
``
228
`+
pats: &[hir::Pat<'_>],
`
``
229
`+
dotdot: Option,
`
``
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
+
223
250
`fn mark_live_symbols(&mut self) {
`
224
251
`let mut scanned = FxHashSet::default();
`
225
252
`while let Some(id) = self.worklist.pop() {
`
`@@ -274,12 +301,15 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
`
274
301
`}
`
275
302
``
276
303
`let had_repr_c = self.repr_has_repr_c;
`
``
304
`+
let had_repr_simd = self.repr_has_repr_simd;
`
277
305
`self.repr_has_repr_c = false;
`
``
306
`+
self.repr_has_repr_simd = false;
`
278
307
`match node {
`
279
308
`Node::Item(item) => match item.kind {
`
280
309
` hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
`
281
310
`let def = self.tcx.adt_def(item.def_id);
`
282
311
`self.repr_has_repr_c = def.repr().c();
`
``
312
`+
self.repr_has_repr_simd = def.repr().simd();
`
283
313
``
284
314
` intravisit::walk_item(self, &item)
`
285
315
`}
`
`@@ -315,6 +345,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
`
315
345
`}
`
316
346
` _ => {}
`
317
347
`}
`
``
348
`+
self.repr_has_repr_simd = had_repr_simd;
`
318
349
`self.repr_has_repr_c = had_repr_c;
`
319
350
`}
`
320
351
``
`@@ -347,9 +378,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
`
347
378
`) {
`
348
379
`let tcx = self.tcx;
`
349
380
`let has_repr_c = self.repr_has_repr_c;
`
``
381
`+
let has_repr_simd = self.repr_has_repr_simd;
`
350
382
`let live_fields = def.fields().iter().filter_map(|f| {
`
351
383
`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) {
`
353
385
`return Some(def_id);
`
354
386
`}
`
355
387
`if !tcx.visibility(f.hir_id.owner).is_public() {
`
`@@ -408,6 +440,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
`
408
440
`let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
`
409
441
`self.handle_res(res);
`
410
442
`}
`
``
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
`+
}
`
411
447
` _ => (),
`
412
448
`}
`
413
449
``
`@@ -440,7 +476,11 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
`
440
476
`}
`
441
477
`}
`
442
478
``
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 {
`
444
484
`let attrs = tcx.hir().attrs(id);
`
445
485
`if tcx.sess.contains_name(attrs, sym::lang) {
`
446
486
`return true;
`
`@@ -470,7 +510,11 @@ fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
`
470
510
`}
`
471
511
`}
`
472
512
``
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)
`
474
518
`}
`
475
519
``
476
520
`// These check_* functions seeds items that
`
`@@ -623,6 +667,7 @@ fn live_symbols_and_ignored_derived_traits<'tcx>(
`
623
667
`maybe_typeck_results: None,
`
624
668
`live_symbols: Default::default(),
`
625
669
`repr_has_repr_c: false,
`
``
670
`+
repr_has_repr_simd: false,
`
626
671
`in_pat: false,
`
627
672
`ignore_variant_stack: vec![],
`
628
673
` struct_constructors,
`
`@@ -644,32 +689,46 @@ struct DeadVisitor<'tcx> {
`
644
689
`ignored_derived_traits: &'tcx FxHashMap<LocalDefId, Vec<(DefId, DefId)>>,
`
645
690
`}
`
646
691
``
``
692
`+
enum ShouldWarnAboutField {
`
``
693
`+
Yes(bool), // positional?
`
``
694
`+
No,
`
``
695
`+
}
`
``
696
+
647
697
`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 {
`
649
699
`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;
`
651
705
`}
`
652
706
`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;
`
655
714
`}
`
656
``
`-
let field_type = self.tcx.type_of(field.did);
`
657
``
`-
!field_type.is_phantom_data()
`
``
715
`+
ShouldWarnAboutField::Yes(is_positional)
`
658
716
`}
`
659
717
``
660
718
`fn warn_multiple_dead_codes(
`
661
719
`&self,
`
662
720
`dead_codes: &[LocalDefId],
`
663
721
`participle: &str,
`
664
722
`parent_item: Option,
`
``
723
`+
is_positional: bool,
`
665
724
`) {
`
666
725
`if let Some(&first_id) = dead_codes.first() {
`
667
726
`let tcx = self.tcx;
`
668
727
`let names: Vec<_> = dead_codes
`
669
728
`.iter()
`
670
729
`.map(|&def_id| tcx.item_name(def_id.to_def_id()).to_string())
`
671
730
`.collect();
`
672
``
`-
let spans = dead_codes
`
``
731
`+
let spans: Vec<_> = dead_codes
`
673
732
`.iter()
`
674
733
`.map(|&def_id| match tcx.def_ident_span(def_id) {
`
675
734
`Some(s) => s.with_ctxt(tcx.def_span(def_id).ctxt()),
`
`@@ -678,9 +737,13 @@ impl<'tcx> DeadVisitor<'tcx> {
`
678
737
`.collect();
`
679
738
``
680
739
` 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
`+
},
`
682
745
` tcx.hir().local_def_id_to_hir_id(first_id),
`
683
``
`-
MultiSpan::from_spans(spans),
`
``
746
`+
MultiSpan::from_spans(spans.clone()),
`
684
747
` |lint| {
`
685
748
`let descr = tcx.def_kind(first_id).descr(first_id.to_def_id());
`
686
749
`let span_len = dead_codes.len();
`
`@@ -702,6 +765,21 @@ impl<'tcx> DeadVisitor<'tcx> {
`
702
765
` are = pluralize!("is", span_len),
`
703
766
`));
`
704
767
``
``
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
+
705
783
`if let Some(parent_item) = parent_item {
`
706
784
`let parent_descr = tcx.def_kind(parent_item).descr(parent_item.to_def_id());
`
707
785
` err.span_label(
`
`@@ -743,6 +821,7 @@ impl<'tcx> DeadVisitor<'tcx> {
`
743
821
`def_id: LocalDefId,
`
744
822
`participle: &str,
`
745
823
`dead_codes: Vec,
`
``
824
`+
is_positional: bool,
`
746
825
`) {
`
747
826
`let mut dead_codes = dead_codes
`
748
827
`.iter()
`
`@@ -758,12 +837,13 @@ impl<'tcx> DeadVisitor<'tcx> {
`
758
837
`&group.map(|v| v.def_id).collect::<Vec<_>>(),
`
759
838
` participle,
`
760
839
`Some(def_id),
`
``
840
`+
is_positional,
`
761
841
`);
`
762
842
`}
`
763
843
`}
`
764
844
``
765
845
`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);
`
767
847
`}
`
768
848
``
769
849
`fn check_definition(&mut self, def_id: LocalDefId) {
`
`@@ -829,24 +909,37 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalDefId) {
`
829
909
`continue;
`
830
910
`}
`
831
911
``
``
912
`+
let mut is_positional = false;
`
832
913
`let dead_fields = variant
`
833
914
`.fields
`
834
915
`.iter()
`
835
916
`.filter_map(|field| {
`
836
917
`let def_id = field.did.expect_local();
`
837
918
`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;
`
840
933
`Some(DeadVariant { def_id, name: field.name, level })
`
841
934
`} else {
`
842
935
`None
`
843
936
`}
`
844
937
`})
`
845
938
`.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)
`
847
940
`}
`
848
941
``
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);
`
850
943
`}
`
851
944
`}
`
852
945
``