Rollup 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`

compiler/rustc_apfloat/src/ppc.rs

@@ -30,7 +30,7 @@ pub type DoubleDouble = DoubleFloat<ieee::Double>;
 // FIXME: Implement all operations in DoubleDouble, and delete these
 // semantics.
 // FIXME(eddyb) This shouldn't need to be `pub`, it's only used in bounds.
-pub struct FallbackS<F>(F);
+pub struct FallbackS<F>(#[allow(unused)] F);
 type Fallback<F> = ieee::IeeeFloat<FallbackS<F>>;
 impl<F: Float> ieee::Semantics for FallbackS<F> {
  // Forbid any conversion to/from bits.
@@ -45,7 +45,7 @@ impl<F: Float> ieee::Semantics for FallbackS<F> {
 // truncate the mantissa. The result of that second conversion
 // may be inexact, but should never underflow.
 // FIXME(eddyb) This shouldn't need to be `pub`, it's only used in bounds.
-pub struct FallbackExtendedS<F>(F);
+pub struct FallbackExtendedS<F>(#[allow(unused)] F);
 type FallbackExtended<F> = ieee::IeeeFloat<FallbackExtendedS<F>>;
 impl<F: Float> ieee::Semantics for FallbackExtendedS<F> {
  // Forbid any conversion to/from bits.

compiler/rustc_lint_defs/src/builtin.rs

@@ -630,6 +630,32 @@ declare_lint! {
  "detects attributes that were not used by the compiler"
 }
 
+declare_lint! {
+ /// The `unused_tuple_struct_fields` lint detects fields of tuple structs
+ /// that are never read.
+ ///
+ /// ### Example
+ ///
+ /// ```
+ /// #[warn(unused_tuple_struct_fields)]
+ /// struct S(i32, i32, i32);
+ /// let s = S(1, 2, 3);
+ /// let _ = (s.0, s.2);
+ /// ```
+ ///
+ /// {{produces}}
+ ///
+ /// ### Explanation
+ ///
+ /// Tuple struct fields that are never read anywhere may indicate a
+ /// mistake or unfinished code. To silence this warning, consider
+ /// removing the unused field(s) or, to preserve the numbering of the
+ /// remaining fields, change the unused field(s) to have unit type.
+ pub UNUSED_TUPLE_STRUCT_FIELDS,
+ Allow,
+ "detects tuple struct fields that are never read"
+}
+
 declare_lint! {
  /// The `unreachable_code` lint detects unreachable code paths.
  ///
@@ -3281,6 +3307,7 @@ declare_lint_pass! {
  UNSUPPORTED_CALLING_CONVENTIONS,
  BREAK_WITH_LABEL_AND_LOOP,
  UNUSED_ATTRIBUTES,
+ UNUSED_TUPLE_STRUCT_FIELDS,
  NON_EXHAUSTIVE_OMITTED_PATTERNS,
  TEXT_DIRECTION_CODEPOINT_IN_COMMENT,
  DEREF_INTO_DYN_SUPERTRAIT,

compiler/rustc_passes/src/check_attr.rs

@@ -53,7 +53,7 @@ pub(crate) fn target_from_impl_item<'tcx>(
 #[derive(Clone, Copy)]
 enum ItemLike<'tcx> {
  Item(&'tcx Item<'tcx>),
- ForeignItem(&'tcx ForeignItem<'tcx>),
+ ForeignItem,
 }
 
 struct CheckAttrVisitor<'tcx> {
@@ -1995,12 +1995,7 @@ impl<'tcx> Visitor<'tcx> for CheckAttrVisitor<'tcx> {
 
  fn visit_foreign_item(&mut self, f_item: &'tcx ForeignItem<'tcx>) {
  let target = Target::from_foreign_item(f_item);
- self.check_attributes(
- f_item.hir_id(),
- f_item.span,
- target,
- Some(ItemLike::ForeignItem(f_item)),
- );
+ self.check_attributes(f_item.hir_id(), f_item.span, target, Some(ItemLike::ForeignItem));
  intravisit::walk_foreign_item(self, f_item)
  }
 

compiler/rustc_passes/src/dead.rs

@@ -4,7 +4,7 @@
 
 use itertools::Itertools;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_errors::{pluralize, MultiSpan};
+use rustc_errors::{pluralize, Applicability, MultiSpan};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorOf, DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId};
@@ -42,6 +42,7 @@ struct MarkSymbolVisitor<'tcx> {
  maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>,
  live_symbols: FxHashSet<LocalDefId>,
  repr_has_repr_c: bool,
+ repr_has_repr_simd: bool,
  in_pat: bool,
  ignore_variant_stack: Vec<DefId>,
  // maps from tuple struct constructors to tuple struct items
@@ -220,6 +221,32 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
  }
  }
 
+ fn handle_tuple_field_pattern_match(
+ &mut self,
+ lhs: &hir::Pat<'_>,
+ res: Res,
+ pats: &[hir::Pat<'_>],
+ dotdot: Option<usize>,
+ ) {
+ let variant = match self.typeck_results().node_type(lhs.hir_id).kind() {
+ ty::Adt(adt, _) => adt.variant_of_res(res),
+ _ => span_bug!(lhs.span, "non-ADT in tuple struct pattern"),
+ };
+ let first_n = pats.iter().enumerate().take(dotdot.unwrap_or(pats.len()));
+ let missing = variant.fields.len() - pats.len();
+ let last_n = pats
+ .iter()
+ .enumerate()
+ .skip(dotdot.unwrap_or(pats.len()))
+ .map(|(idx, pat)| (idx + missing, pat));
+ for (idx, pat) in first_n.chain(last_n) {
+ if let PatKind::Wild = pat.kind {
+ continue;
+ }
+ self.insert_def_id(variant.fields[idx].did);
+ }
+ }
+
  fn mark_live_symbols(&mut self) {
  let mut scanned = FxHashSet::default();
  while let Some(id) = self.worklist.pop() {
@@ -274,12 +301,15 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
  }
 
  let had_repr_c = self.repr_has_repr_c;
+ let had_repr_simd = self.repr_has_repr_simd;
  self.repr_has_repr_c = false;
+ self.repr_has_repr_simd = false;
  match node {
  Node::Item(item) => match item.kind {
  hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
  let def = self.tcx.adt_def(item.def_id);
  self.repr_has_repr_c = def.repr().c();
+ self.repr_has_repr_simd = def.repr().simd();
 
  intravisit::walk_item(self, &item)
  }
@@ -315,6 +345,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
  }
  _ => {}
  }
+ self.repr_has_repr_simd = had_repr_simd;
  self.repr_has_repr_c = had_repr_c;
  }
 
@@ -347,9 +378,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
  ) {
  let tcx = self.tcx;
  let has_repr_c = self.repr_has_repr_c;
+ let has_repr_simd = self.repr_has_repr_simd;
  let live_fields = def.fields().iter().filter_map(|f| {
  let def_id = tcx.hir().local_def_id(f.hir_id);
- if has_repr_c {
+ if has_repr_c || (f.is_positional() && has_repr_simd) {
  return Some(def_id);
  }
  if !tcx.visibility(f.hir_id.owner).is_public() {
@@ -408,6 +440,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
  let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
  self.handle_res(res);
  }
+ PatKind::TupleStruct(ref qpath, ref fields, dotdot) => {
+ let res = self.typeck_results().qpath_res(qpath, pat.hir_id);
+ self.handle_tuple_field_pattern_match(pat, res, fields, dotdot);
+ }
  _ => (),
  }
 
@@ -440,7 +476,11 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
  }
 }
 
-fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
+fn has_allow_dead_code_or_lang_attr_helper(
+ tcx: TyCtxt<'_>,
+ id: hir::HirId,
+ lint: &'static lint::Lint,
+) -> bool {
  let attrs = tcx.hir().attrs(id);
  if tcx.sess.contains_name(attrs, sym::lang) {
  return true;
@@ -470,7 +510,11 @@ fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
  }
  }
 
- tcx.lint_level_at_node(lint::builtin::DEAD_CODE, id).0 == lint::Allow
+ tcx.lint_level_at_node(lint, id).0 == lint::Allow
+}
+
+fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool {
+ has_allow_dead_code_or_lang_attr_helper(tcx, id, lint::builtin::DEAD_CODE)
 }
 
 // These check_* functions seeds items that
@@ -623,6 +667,7 @@ fn live_symbols_and_ignored_derived_traits<'tcx>(
  maybe_typeck_results: None,
  live_symbols: Default::default(),
  repr_has_repr_c: false,
+ repr_has_repr_simd: false,
  in_pat: false,
  ignore_variant_stack: vec![],
  struct_constructors,
@@ -644,32 +689,46 @@ struct DeadVisitor<'tcx> {
  ignored_derived_traits: &'tcx FxHashMap<LocalDefId, Vec<(DefId, DefId)>>,
 }
 
+enum ShouldWarnAboutField {
+ Yes(bool), // positional?
+ No,
+}
+
 impl<'tcx> DeadVisitor<'tcx> {
- fn should_warn_about_field(&mut self, field: &ty::FieldDef) -> bool {
+ fn should_warn_about_field(&mut self, field: &ty::FieldDef) -> ShouldWarnAboutField {
  if self.live_symbols.contains(&field.did.expect_local()) {
- return false;
+ return ShouldWarnAboutField::No;
+ }
+ let field_type = self.tcx.type_of(field.did);
+ if field_type.is_phantom_data() {
+ return ShouldWarnAboutField::No;
  }
  let is_positional = field.name.as_str().starts_with(|c: char| c.is_ascii_digit());
- if is_positional {
- return false;
+ if is_positional
+ && self
+ .tcx
+ .layout_of(self.tcx.param_env(field.did).and(field_type))
+ .map_or(true, |layout| layout.is_zst())
+ {
+ return ShouldWarnAboutField::No;
  }
- let field_type = self.tcx.type_of(field.did);
- !field_type.is_phantom_data()
+ ShouldWarnAboutField::Yes(is_positional)
  }
 
  fn warn_multiple_dead_codes(
  &self,
  dead_codes: &[LocalDefId],
  participle: &str,
  parent_item: Option<LocalDefId>,
+ is_positional: bool,
  ) {
  if let Some(&first_id) = dead_codes.first() {
  let tcx = self.tcx;
  let names: Vec<_> = dead_codes
  .iter()
  .map(|&def_id| tcx.item_name(def_id.to_def_id()).to_string())
  .collect();
- let spans = dead_codes
+ let spans: Vec<_> = dead_codes
  .iter()
  .map(|&def_id| match tcx.def_ident_span(def_id) {
  Some(s) => s.with_ctxt(tcx.def_span(def_id).ctxt()),
@@ -678,9 +737,13 @@ impl<'tcx> DeadVisitor<'tcx> {
  .collect();
 
  tcx.struct_span_lint_hir(
- lint::builtin::DEAD_CODE,
+ if is_positional {
+ lint::builtin::UNUSED_TUPLE_STRUCT_FIELDS
+ } else {
+ lint::builtin::DEAD_CODE
+ },
  tcx.hir().local_def_id_to_hir_id(first_id),
- MultiSpan::from_spans(spans),
+ MultiSpan::from_spans(spans.clone()),
  |lint| {
  let descr = tcx.def_kind(first_id).descr(first_id.to_def_id());
  let span_len = dead_codes.len();
@@ -702,6 +765,21 @@ impl<'tcx> DeadVisitor<'tcx> {
  are = pluralize!("is", span_len),
  ));
 
+ if is_positional {
+ err.multipart_suggestion(
+ &format!(
+ "consider changing the field{s} to be of unit type to \
+ suppress this warning while preserving the field \
+ numbering, or remove the field{s}",
+ s = pluralize!(span_len)
+ ),
+ spans.iter().map(|sp| (*sp, "()".to_string())).collect(),
+ // "HasPlaceholders" because applying this fix by itself isn't
+ // enough: All constructor calls have to be adjusted as well
+ Applicability::HasPlaceholders,
+ );
+ }
+
  if let Some(parent_item) = parent_item {
  let parent_descr = tcx.def_kind(parent_item).descr(parent_item.to_def_id());
  err.span_label(
@@ -743,6 +821,7 @@ impl<'tcx> DeadVisitor<'tcx> {
  def_id: LocalDefId,
  participle: &str,
  dead_codes: Vec<DeadVariant>,
+ is_positional: bool,
  ) {
  let mut dead_codes = dead_codes
  .iter()
@@ -758,12 +837,13 @@ impl<'tcx> DeadVisitor<'tcx> {
  &group.map(|v| v.def_id).collect::<Vec<_>>(),
  participle,
  Some(def_id),
+ is_positional,
  );
  }
  }
 
  fn warn_dead_code(&mut self, id: LocalDefId, participle: &str) {
- self.warn_multiple_dead_codes(&[id], participle, None);
+ self.warn_multiple_dead_codes(&[id], participle, None, false);
  }
 
  fn check_definition(&mut self, def_id: LocalDefId) {
@@ -829,24 +909,37 @@ fn check_mod_deathness(tcx: TyCtxt<'_>, module: LocalDefId) {
  continue;
  }
 
+ let mut is_positional = false;
  let dead_fields = variant
  .fields
  .iter()
  .filter_map(|field| {
  let def_id = field.did.expect_local();
  let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
- if visitor.should_warn_about_field(&field) {
- let level = tcx.lint_level_at_node(lint::builtin::DEAD_CODE, hir_id).0;
+ if let ShouldWarnAboutField::Yes(is_pos) =
+ visitor.should_warn_about_field(&field)
+ {
+ let level = tcx
+ .lint_level_at_node(
+ if is_pos {
+ is_positional = true;
+ lint::builtin::UNUSED_TUPLE_STRUCT_FIELDS
+ } else {
+ lint::builtin::DEAD_CODE
+ },
+ hir_id,
+ )
+ .0;
  Some(DeadVariant { def_id, name: field.name, level })
  } else {
  None
  }
  })
  .collect();
- visitor.warn_dead_fields_and_variants(def_id, "read", dead_fields)
+ visitor.warn_dead_fields_and_variants(def_id, "read", dead_fields, is_positional)
  }
 
- visitor.warn_dead_fields_and_variants(item.def_id, "constructed", dead_variants);
+ visitor.warn_dead_fields_and_variants(item.def_id, "constructed", dead_variants, false);
  }
  }
 

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

@@ -34,9 +34,9 @@ enum EnumNoDrop<T1, T2> {
 }
 
 
-struct NonGenericNoDrop(i32);
+struct NonGenericNoDrop(#[allow(unused_tuple_struct_fields)] i32);
 
-struct NonGenericWithDrop(i32);
+struct NonGenericWithDrop(#[allow(unused_tuple_struct_fields)] i32);
 //~ MONO_ITEM fn std::ptr::drop_in_place::<NonGenericWithDrop> - shim(Some(NonGenericWithDrop)) @@ generic_drop_glue-cgu.0[Internal]
 
 impl Drop for NonGenericWithDrop {