Skip to content

Commit

Permalink
Rollup merge of rust-lang#124580 - gurry:124556-suggest-remove-tuple-…
Browse files Browse the repository at this point in the history
…field, r=jackh726

Suggest removing unused tuple fields if they are the last fields

Fixes rust-lang#124556

We now check if dead/unused fields are the last fields of the tuple and suggest their removal instead of suggesting them to be changed to `()`.
  • Loading branch information
GuillaumeGomez authored Jun 18, 2024
2 parents 3b7bb08 + 012a458 commit b0237bd
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 45 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,15 @@ passes_pass_by_value =
passes_proc_macro_bad_sig = {$kind} has incorrect signature
passes_remove_fields =
consider removing { $num ->
[one] this
*[other] these
} { $num ->
[one] field
*[other] fields
}
passes_repr_conflicting =
conflicting representation hints
Expand Down
58 changes: 45 additions & 13 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ use rustc_target::abi::FieldIdx;
use std::mem;

use crate::errors::{
ChangeFieldsToBeOfUnitType, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo,
UselessAssignment,
ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment,
};

// Any local node that may call something in its body block should be
Expand Down Expand Up @@ -1071,17 +1070,50 @@ impl<'tcx> DeadVisitor<'tcx> {
};

let diag = match report_on {
ReportOn::TupleField => MultipleDeadCodes::UnusedTupleStructFields {
multiple,
num,
descr,
participle,
name_list,
change_fields_suggestion: ChangeFieldsToBeOfUnitType { num, spans: spans.clone() },
parent_info,
ignored_derived_impls,
},

ReportOn::TupleField => {
let tuple_fields = if let Some(parent_id) = parent_item
&& let node = tcx.hir_node_by_def_id(parent_id)
&& let hir::Node::Item(hir::Item {
kind: hir::ItemKind::Struct(hir::VariantData::Tuple(fields, _, _), _),
..
}) = node
{
*fields
} else {
&[]
};

let trailing_tuple_fields = if tuple_fields.len() >= dead_codes.len() {
LocalDefIdSet::from_iter(
tuple_fields
.iter()
.skip(tuple_fields.len() - dead_codes.len())
.map(|f| f.def_id),
)
} else {
LocalDefIdSet::default()
};

let fields_suggestion =
// Suggest removal if all tuple fields are at the end.
// Otherwise suggest removal or changing to unit type
if dead_codes.iter().all(|dc| trailing_tuple_fields.contains(&dc.def_id)) {
ChangeFields::Remove { num }
} else {
ChangeFields::ChangeToUnitTypeOrRemove { num, spans: spans.clone() }
};

MultipleDeadCodes::UnusedTupleStructFields {
multiple,
num,
descr,
participle,
name_list,
change_fields_suggestion: fields_suggestion,
parent_info,
ignored_derived_impls,
}
}
ReportOn::NamedField => MultipleDeadCodes::DeadCodes {
multiple,
num,
Expand Down
19 changes: 13 additions & 6 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1574,7 +1574,7 @@ pub enum MultipleDeadCodes<'tcx> {
participle: &'tcx str,
name_list: DiagSymbolList,
#[subdiagnostic]
change_fields_suggestion: ChangeFieldsToBeOfUnitType,
change_fields_suggestion: ChangeFields,
#[subdiagnostic]
parent_info: Option<ParentInfo<'tcx>>,
#[subdiagnostic]
Expand All @@ -1601,11 +1601,18 @@ pub struct IgnoredDerivedImpls {
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(passes_change_fields_to_be_of_unit_type, applicability = "has-placeholders")]
pub struct ChangeFieldsToBeOfUnitType {
pub num: usize,
#[suggestion_part(code = "()")]
pub spans: Vec<Span>,
pub enum ChangeFields {
#[multipart_suggestion(
passes_change_fields_to_be_of_unit_type,
applicability = "has-placeholders"
)]
ChangeToUnitTypeOrRemove {
num: usize,
#[suggestion_part(code = "()")]
spans: Vec<Span>,
},
#[help(passes_remove_fields)]
Remove { num: usize },
}

#[derive(Diagnostic)]
Expand Down
31 changes: 20 additions & 11 deletions tests/ui/lint/dead-code/tuple-struct-field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@ use std::marker::PhantomData;

const LEN: usize = 4;

struct SingleUnused(i32, [u8; LEN], String);
//~^ ERROR: field `1` is never read
struct UnusedAtTheEnd(i32, f32, [u8; LEN], String, u8);
//~^ ERROR:fields `1`, `2`, `3`, and `4` are never read
//~| NOTE: fields in this struct
//~| HELP: consider removing these fields

struct UnusedJustOneField(i32);
//~^ ERROR: field `0` is never read
//~| NOTE: field in this struct
//~| HELP: consider changing the field to be of unit type
//~| HELP: consider removing this field

struct MultipleUnused(i32, f32, String, u8);
//~^ ERROR: fields `0`, `1`, `2`, and `3` are never read
struct UnusedInTheMiddle(i32, f32, String, u8, u32);
//~^ ERROR: fields `1`, `2`, and `4` are never read
//~| NOTE: fields in this struct
//~| HELP: consider changing the fields to be of unit type
//~| HELP: consider changing the fields to be of unit type to suppress this warning while preserving the field numbering, or remove the fields

struct GoodUnit(());

Expand All @@ -23,15 +28,19 @@ struct Void;
struct GoodVoid(Void);

fn main() {
let w = SingleUnused(42, [0, 1, 2, 3], "abc".to_string());
let _ = w.0;
let _ = w.2;
let u1 = UnusedAtTheEnd(42, 3.14, [0, 1, 2, 3], "def".to_string(), 4u8);
let _ = u1.0;

let _ = UnusedJustOneField(42);

let u2 = UnusedInTheMiddle(42, 3.14, "def".to_string(), 4u8, 5);
let _ = u2.0;
let _ = u2.3;

let m = MultipleUnused(42, 3.14, "def".to_string(), 4u8);

let gu = GoodUnit(());
let gp = GoodPhantom(PhantomData);
let gv = GoodVoid(Void);

let _ = (gu, gp, gv, m);
let _ = (gu, gp, gv);
}
37 changes: 22 additions & 15 deletions tests/ui/lint/dead-code/tuple-struct-field.stderr
Original file line number Diff line number Diff line change
@@ -1,33 +1,40 @@
error: field `1` is never read
--> $DIR/tuple-struct-field.rs:8:26
error: fields `1`, `2`, `3`, and `4` are never read
--> $DIR/tuple-struct-field.rs:8:28
|
LL | struct SingleUnused(i32, [u8; LEN], String);
| ------------ ^^^^^^^^^
LL | struct UnusedAtTheEnd(i32, f32, [u8; LEN], String, u8);
| -------------- ^^^ ^^^^^^^^^ ^^^^^^ ^^
| |
| field in this struct
| fields in this struct
|
= help: consider removing these fields
note: the lint level is defined here
--> $DIR/tuple-struct-field.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field

error: field `0` is never read
--> $DIR/tuple-struct-field.rs:13:27
|
LL | struct UnusedJustOneField(i32);
| ------------------ ^^^
| |
| field in this struct
|
LL | struct SingleUnused(i32, (), String);
| ~~
= help: consider removing this field

error: fields `0`, `1`, `2`, and `3` are never read
--> $DIR/tuple-struct-field.rs:13:23
error: fields `1`, `2`, and `4` are never read
--> $DIR/tuple-struct-field.rs:18:31
|
LL | struct MultipleUnused(i32, f32, String, u8);
| -------------- ^^^ ^^^ ^^^^^^ ^^
LL | struct UnusedInTheMiddle(i32, f32, String, u8, u32);
| ----------------- ^^^ ^^^^^^ ^^^
| |
| fields in this struct
|
help: consider changing the fields to be of unit type to suppress this warning while preserving the field numbering, or remove the fields
|
LL | struct MultipleUnused((), (), (), ());
| ~~ ~~ ~~ ~~
LL | struct UnusedInTheMiddle(i32, (), (), u8, ());
| ~~ ~~ ~~

error: aborting due to 2 previous errors
error: aborting due to 3 previous errors

0 comments on commit b0237bd

Please sign in to comment.