Skip to content

Commit e22e443

Browse files
sapirsapir
committed
Try to fix warning for unused variables in or patterns, issue rust-lang#67691 
1 parent 0c156af commit e22e443

8 files changed

+140
-57
lines changed

‎src/librustc_passes/liveness.rs

+45-26
Original file line numberDiff line numberDiff line change
@@ -1492,28 +1492,33 @@ impl<'tcx> Liveness<'_, 'tcx> {
14921492
) {
14931493
// In an or-pattern, only consider the variable; any later patterns must have the same
14941494
// bindings, and we also consider the first pattern to be the "authoritative" set of ids.
1495-
// However, we should take the spans of variables with the same name from the later
1495+
// However, we should take the ids and spans of variables with the same name from the later
14961496
// patterns so the suggestions to prefix with underscores will apply to those too.
1497-
let mut vars: FxIndexMap<String, (LiveNode, Variable, HirId, Vec<Span>)> = <_>::default();
1497+
let mut vars: FxIndexMap<String, (LiveNode, Variable, Vec<(HirId, Span)>)> = <_>::default();
14981498

14991499
pat.each_binding(|_, hir_id, pat_sp, ident| {
15001500
let ln = entry_ln.unwrap_or_else(|| self.live_node(hir_id, pat_sp));
15011501
let var = self.variable(hir_id, ident.span);
1502+
let id_and_sp = (hir_id, pat_sp);
15021503
vars.entry(self.ir.variable_name(var))
1503-
.and_modify(|(.., spans)| spans.push(ident.span))
1504-
.or_insert_with(|| (ln, var, hir_id, vec![ident.span]));
1504+
.and_modify(|(.., hir_ids_and_spans)| hir_ids_and_spans.push(id_and_sp))
1505+
.or_insert_with(|| (ln, var, vec![id_and_sp]));
15051506
});
15061507

1507-
for (_, (ln, var, id, spans)) in vars {
1508+
for (_, (ln, var, hir_ids_and_spans)) in vars {
15081509
if self.used_on_entry(ln, var) {
1510+
let id = hir_ids_and_spans[0].0;
1511+
let spans = hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect();
15091512
on_used_on_entry(spans, id, ln, var);
15101513
} else {
1511-
self.report_unused(spans, id, ln, var);
1514+
self.report_unused(hir_ids_and_spans, ln, var);
15121515
}
15131516
}
15141517
}
15151518

1516-
fn report_unused(&self, spans: Vec<Span>, hir_id: HirId, ln: LiveNode, var: Variable) {
1519+
fn report_unused(&self, hir_ids_and_spans: Vec<(HirId, Span)>, ln: LiveNode, var: Variable) {
1520+
let first_hir_id = hir_ids_and_spans[0].0;
1521+
15171522
if let Some(name) = self.should_warn(var).filter(|name| name != "self") {
15181523
// annoying: for parameters in funcs like `fn(x: i32)
15191524
// {ret}`, there is only one node, so asking about
@@ -1524,8 +1529,8 @@ impl<'tcx> Liveness<'_, 'tcx> {
15241529
if is_assigned {
15251530
self.ir.tcx.struct_span_lint_hir(
15261531
lint::builtin::UNUSED_VARIABLES,
1527-
hir_id,
1528-
spans,
1532+
first_hir_id,
1533+
hir_ids_and_spans.into_iter().map(|(_, sp)| sp).collect::<Vec<_>>(),
15291534
|lint| {
15301535
lint.build(&format!("variable `{}` is assigned to, but never used", name))
15311536
.note(&format!("consider using `_{}` instead", name))
@@ -1535,31 +1540,45 @@ impl<'tcx> Liveness<'_, 'tcx> {
15351540
} else {
15361541
self.ir.tcx.struct_span_lint_hir(
15371542
lint::builtin::UNUSED_VARIABLES,
1538-
hir_id,
1539-
spans.clone(),
1543+
first_hir_id,
1544+
hir_ids_and_spans.iter().map(|(_, sp)| *sp).collect::<Vec<_>>(),
15401545
|lint| {
15411546
let mut err = lint.build(&format!("unused variable: `{}`", name));
1542-
if self.ir.variable_is_shorthand(var) {
1543-
if let Node::Binding(pat) = self.ir.tcx.hir().get(hir_id) {
1544-
// Handle `ref` and `ref mut`.
1545-
let spans = spans
1546-
.iter()
1547-
.map(|_span| (pat.span, format!("{}: _", name)))
1548-
.collect();
1549-
1550-
err.multipart_suggestion(
1551-
"try ignoring the field",
1552-
spans,
1553-
Applicability::MachineApplicable,
1554-
);
1555-
}
1547+
1548+
let (shorthands, non_shorthands): (Vec<_>, Vec<_>) =
1549+
hir_ids_and_spans.into_iter().partition(|(hir_id, span)| {
1550+
let var = self.variable(*hir_id, *span);
1551+
self.ir.variable_is_shorthand(var)
1552+
});
1553+
1554+
let mut shorthands = shorthands
1555+
.into_iter()
1556+
.map(|(_, span)| (span, format!("{}: _", name)))
1557+
.collect::<Vec<_>>();
1558+
1559+
let non_shorthands = non_shorthands
1560+
.into_iter()
1561+
.map(|(_, span)| (span, format!("_{}", name)))
1562+
.collect::<Vec<_>>();
1563+
1564+
// If we have both shorthand and non-shorthand, prefer the "try ignoring
1565+
// the field" message.
1566+
if !shorthands.is_empty() {
1567+
shorthands.extend(non_shorthands);
1568+
1569+
err.multipart_suggestion(
1570+
"try ignoring the field",
1571+
shorthands,
1572+
Applicability::MachineApplicable,
1573+
);
15561574
} else {
15571575
err.multipart_suggestion(
15581576
"if this is intentional, prefix it with an underscore",
1559-
spans.iter().map(|span| (*span, format!("_{}", name))).collect(),
1577+
non_shorthands,
15601578
Applicability::MachineApplicable,
15611579
);
15621580
}
1581+
15631582
err.emit()
15641583
},
15651584
);

‎src/test/ui/lint/issue-47390-unused-variable-in-struct-pattern.stderr

+6-6
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,16 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
1212
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`
1313

1414
warning: unused variable: `mut_unused_var`
15-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13
15+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:9
1616
|
1717
LL | let mut mut_unused_var = 1;
18-
| ^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var`
18+
| ^^^^^^^^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_mut_unused_var`
1919

2020
warning: unused variable: `var`
21-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:14
21+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:10
2222
|
2323
LL | let (mut var, unused_var) = (1, 2);
24-
| ^^^ help: if this is intentional, prefix it with an underscore: `_var`
24+
| ^^^^^^^ help: if this is intentional, prefix it with an underscore: `_var`
2525

2626
warning: unused variable: `unused_var`
2727
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:19
@@ -36,10 +36,10 @@ LL | if let SoulHistory { corridors_of_light,
3636
| ^^^^^^^^^^^^^^^^^^ help: try ignoring the field: `corridors_of_light: _`
3737

3838
warning: variable `hours_are_suns` is assigned to, but never used
39-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:30
39+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:46:26
4040
|
4141
LL | mut hours_are_suns,
42-
| ^^^^^^^^^^^^^^
42+
| ^^^^^^^^^^^^^^^^^^
4343
|
4444
= note: consider using `_hours_are_suns` instead
4545

‎src/test/ui/lint/issue-54180-unused-ref-field.stderr

+6-12
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
error: unused variable: `field`
2-
--> $DIR/issue-54180-unused-ref-field.rs:20:26
2+
--> $DIR/issue-54180-unused-ref-field.rs:20:22
33
|
44
LL | E::Variant { ref field } => (),
5-
| ----^^^^^
6-
| |
7-
| help: try ignoring the field: `field: _`
5+
| ^^^^^^^^^ help: try ignoring the field: `field: _`
86
|
97
note: the lint level is defined here
108
--> $DIR/issue-54180-unused-ref-field.rs:3:9
@@ -20,20 +18,16 @@ LL | let _: i32 = points.iter().map(|Point { x, y }| y).sum();
2018
| ^ help: try ignoring the field: `x: _`
2119

2220
error: unused variable: `f1`
23-
--> $DIR/issue-54180-unused-ref-field.rs:26:17
21+
--> $DIR/issue-54180-unused-ref-field.rs:26:13
2422
|
2523
LL | let S { ref f1 } = s;
26-
| ----^^
27-
| |
28-
| help: try ignoring the field: `f1: _`
24+
| ^^^^^^ help: try ignoring the field: `f1: _`
2925

3026
error: unused variable: `x`
31-
--> $DIR/issue-54180-unused-ref-field.rs:32:28
27+
--> $DIR/issue-54180-unused-ref-field.rs:32:20
3228
|
3329
LL | Point { y, ref mut x } => y,
34-
| --------^
35-
| |
36-
| help: try ignoring the field: `x: _`
30+
| ^^^^^^^^^ help: try ignoring the field: `x: _`
3731

3832
error: aborting due to 4 previous errors
3933

‎src/test/ui/lint/issue-67691-unused-field-in-or-pattern.fixed

+24
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ pub enum MyEnum {
88
B { i: i32, j: i32 },
99
}
1010

11+
pub enum MixedEnum {
12+
A { i: i32 },
13+
B(i32),
14+
}
15+
1116
pub fn no_ref(x: MyEnum) {
1217
use MyEnum::*;
1318

@@ -52,10 +57,29 @@ pub fn inner_with_ref(x: Option<MyEnum>) {
5257
}
5358
}
5459

60+
pub fn mixed_no_ref(x: MixedEnum) {
61+
match x {
62+
MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
63+
println!("match");
64+
}
65+
}
66+
}
67+
68+
pub fn mixed_with_ref(x: MixedEnum) {
69+
match x {
70+
MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
71+
println!("match");
72+
}
73+
}
74+
}
75+
5576
pub fn main() {
5677
no_ref(MyEnum::A { i: 1, j: 2 });
5778
with_ref(MyEnum::A { i: 1, j: 2 });
5879

5980
inner_no_ref(Some(MyEnum::A { i: 1, j: 2 }));
6081
inner_with_ref(Some(MyEnum::A { i: 1, j: 2 }));
82+
83+
mixed_no_ref(MixedEnum::B(5));
84+
mixed_with_ref(MixedEnum::B(5));
6185
}

‎src/test/ui/lint/issue-67691-unused-field-in-or-pattern.rs

+24
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ pub enum MyEnum {
88
B { i: i32, j: i32 },
99
}
1010

11+
pub enum MixedEnum {
12+
A { i: i32 },
13+
B(i32),
14+
}
15+
1116
pub fn no_ref(x: MyEnum) {
1217
use MyEnum::*;
1318

@@ -52,10 +57,29 @@ pub fn inner_with_ref(x: Option<MyEnum>) {
5257
}
5358
}
5459

60+
pub fn mixed_no_ref(x: MixedEnum) {
61+
match x {
62+
MixedEnum::A { i } | MixedEnum::B(i) => { //~ ERROR unused variable
63+
println!("match");
64+
}
65+
}
66+
}
67+
68+
pub fn mixed_with_ref(x: MixedEnum) {
69+
match x {
70+
MixedEnum::A { ref i } | MixedEnum::B(ref i) => { //~ ERROR unused variable
71+
println!("match");
72+
}
73+
}
74+
}
75+
5576
pub fn main() {
5677
no_ref(MyEnum::A { i: 1, j: 2 });
5778
with_ref(MyEnum::A { i: 1, j: 2 });
5879

5980
inner_no_ref(Some(MyEnum::A { i: 1, j: 2 }));
6081
inner_with_ref(Some(MyEnum::A { i: 1, j: 2 }));
82+
83+
mixed_no_ref(MixedEnum::B(5));
84+
mixed_with_ref(MixedEnum::B(5));
6185
}

‎src/test/ui/lint/issue-67691-unused-field-in-or-pattern.stderr

+27-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: unused variable: `j`
2-
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:15:16
2+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:20:16
33
|
44
LL | A { i, j } | B { i, j } => {
55
| ^ ^
@@ -16,7 +16,7 @@ LL | A { i, j: _ } | B { i, j: _ } => {
1616
| ^^^^ ^^^^
1717

1818
error: unused variable: `j`
19-
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:25:16
19+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:30:16
2020
|
2121
LL | A { i, ref j } | B { i, ref j } => {
2222
| ^^^^^ ^^^^^
@@ -27,7 +27,7 @@ LL | A { i, j: _ } | B { i, j: _ } => {
2727
| ^^^^ ^^^^
2828

2929
error: unused variable: `j`
30-
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:35:21
30+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:40:21
3131
|
3232
LL | Some(A { i, j } | B { i, j }) => {
3333
| ^ ^
@@ -38,7 +38,7 @@ LL | Some(A { i, j: _ } | B { i, j: _ }) => {
3838
| ^^^^ ^^^^
3939

4040
error: unused variable: `j`
41-
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:47:21
41+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:52:21
4242
|
4343
LL | Some(A { i, ref j } | B { i, ref j }) => {
4444
| ^^^^^ ^^^^^
@@ -48,5 +48,27 @@ help: try ignoring the field
4848
LL | Some(A { i, j: _ } | B { i, j: _ }) => {
4949
| ^^^^ ^^^^
5050

51-
error: aborting due to 5 previous errors
51+
error: unused variable: `i`
52+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:62:24
53+
|
54+
LL | MixedEnum::A { i } | MixedEnum::B(i) => {
55+
| ^ ^
56+
|
57+
help: try ignoring the field
58+
|
59+
LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
60+
| ^^^^ ^^
61+
62+
error: unused variable: `i`
63+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:70:24
64+
|
65+
LL | MixedEnum::A { ref i } | MixedEnum::B(ref i) => {
66+
| ^^^^^ ^^^^^
67+
|
68+
help: try ignoring the field
69+
|
70+
LL | MixedEnum::A { i: _ } | MixedEnum::B(_i) => {
71+
| ^^^^ ^^
72+
73+
error: aborting due to 6 previous errors
5274

‎src/test/ui/liveness/liveness-dead.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: value assigned to `x` is never read
2-
--> $DIR/liveness-dead.rs:9:13
2+
--> $DIR/liveness-dead.rs:9:9
33
|
44
LL | let mut x: isize = 3;
5-
| ^
5+
| ^^^^^
66
|
77
note: the lint level is defined here
88
--> $DIR/liveness-dead.rs:2:9
@@ -20,10 +20,10 @@ LL | x = 4;
2020
= help: maybe it is overwritten before being read?
2121

2222
error: value passed to `x` is never read
23-
--> $DIR/liveness-dead.rs:20:11
23+
--> $DIR/liveness-dead.rs:20:7
2424
|
2525
LL | fn f4(mut x: i32) {
26-
| ^
26+
| ^^^^^
2727
|
2828
= help: maybe it is overwritten before being read?
2929

‎src/test/ui/liveness/liveness-unused.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ LL | let x = 3;
4444
| ^ help: if this is intentional, prefix it with an underscore: `_x`
4545

4646
error: variable `x` is assigned to, but never used
47-
--> $DIR/liveness-unused.rs:30:13
47+
--> $DIR/liveness-unused.rs:30:9
4848
|
4949
LL | let mut x = 3;
50-
| ^
50+
| ^^^^^
5151
|
5252
= note: consider using `_x` instead
5353

@@ -65,10 +65,10 @@ LL | #![deny(unused_assignments)]
6565
= help: maybe it is overwritten before being read?
6666

6767
error: variable `z` is assigned to, but never used
68-
--> $DIR/liveness-unused.rs:37:13
68+
--> $DIR/liveness-unused.rs:37:9
6969
|
7070
LL | let mut z = 3;
71-
| ^
71+
| ^^^^^
7272
|
7373
= note: consider using `_z` instead
7474

0 commit comments

Comments
 (0)