Skip to content

Commit 6947dec

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of #67766 - sapir:fix-unused-in-or-pattern-warning, r=matthewjasper
Fix warning for unused variables in or pattern (issue #67691) Is this a good way to fix it? Also, the tests fail, the "fixed" code output says `{ i, j }` instead of `{ i, j: _ }`, how can I fix that?
2 parents 3712e11 + 3221638 commit 6947dec

File tree

7 files changed

+229
-52
lines changed

7 files changed

+229
-52
lines changed

‎src/librustc_passes/liveness.rs‎

Lines changed: 49 additions & 26 deletions
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,49 @@ 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+
// If we have both shorthand and non-shorthand, prefer the "try ignoring
1560+
// the field" message, and suggest `_` for the non-shorthands. If we only
1561+
// have non-shorthand, then prefix with an underscore instead.
1562+
if !shorthands.is_empty() {
1563+
shorthands.extend(
1564+
non_shorthands
1565+
.into_iter()
1566+
.map(|(_, span)| (span, "_".to_string()))
1567+
.collect::<Vec<_>>(),
1568+
);
1569+
1570+
err.multipart_suggestion(
1571+
"try ignoring the field",
1572+
shorthands,
1573+
Applicability::MachineApplicable,
1574+
);
15561575
} else {
15571576
err.multipart_suggestion(
15581577
"if this is intentional, prefix it with an underscore",
1559-
spans.iter().map(|span| (*span, format!("_{}", name))).collect(),
1578+
non_shorthands
1579+
.into_iter()
1580+
.map(|(_, span)| (span, format!("_{}", name)))
1581+
.collect::<Vec<_>>(),
15601582
Applicability::MachineApplicable,
15611583
);
15621584
}
1585+
15631586
err.emit()
15641587
},
15651588
);

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

Lines changed: 6 additions & 6 deletions
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‎

Lines changed: 6 additions & 12 deletions
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.rs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// FIXME: should be run-rustfix, but rustfix doesn't currently support multipart suggestions, see
2+
// #53934
3+
4+
#![feature(or_patterns)]
5+
#![deny(unused)]
6+
7+
pub enum MyEnum {
8+
A { i: i32, j: i32 },
9+
B { i: i32, j: i32 },
10+
}
11+
12+
pub enum MixedEnum {
13+
A { i: i32 },
14+
B(i32),
15+
}
16+
17+
pub fn no_ref(x: MyEnum) {
18+
use MyEnum::*;
19+
20+
match x {
21+
A { i, j } | B { i, j } => { //~ ERROR unused variable
22+
println!("{}", i);
23+
}
24+
}
25+
}
26+
27+
pub fn with_ref(x: MyEnum) {
28+
use MyEnum::*;
29+
30+
match x {
31+
A { i, ref j } | B { i, ref j } => { //~ ERROR unused variable
32+
println!("{}", i);
33+
}
34+
}
35+
}
36+
37+
pub fn inner_no_ref(x: Option<MyEnum>) {
38+
use MyEnum::*;
39+
40+
match x {
41+
Some(A { i, j } | B { i, j }) => { //~ ERROR unused variable
42+
println!("{}", i);
43+
}
44+
45+
_ => {}
46+
}
47+
}
48+
49+
pub fn inner_with_ref(x: Option<MyEnum>) {
50+
use MyEnum::*;
51+
52+
match x {
53+
Some(A { i, ref j } | B { i, ref j }) => { //~ ERROR unused variable
54+
println!("{}", i);
55+
}
56+
57+
_ => {}
58+
}
59+
}
60+
61+
pub fn mixed_no_ref(x: MixedEnum) {
62+
match x {
63+
MixedEnum::A { i } | MixedEnum::B(i) => { //~ ERROR unused variable
64+
println!("match");
65+
}
66+
}
67+
}
68+
69+
pub fn mixed_with_ref(x: MixedEnum) {
70+
match x {
71+
MixedEnum::A { ref i } | MixedEnum::B(ref i) => { //~ ERROR unused variable
72+
println!("match");
73+
}
74+
}
75+
}
76+
77+
pub fn main() {
78+
no_ref(MyEnum::A { i: 1, j: 2 });
79+
with_ref(MyEnum::A { i: 1, j: 2 });
80+
81+
inner_no_ref(Some(MyEnum::A { i: 1, j: 2 }));
82+
inner_with_ref(Some(MyEnum::A { i: 1, j: 2 }));
83+
84+
mixed_no_ref(MixedEnum::B(5));
85+
mixed_with_ref(MixedEnum::B(5));
86+
}

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
error: unused variable: `j`
2+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:21:16
3+
|
4+
LL | A { i, j } | B { i, j } => {
5+
| ^ ^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:5:9
9+
|
10+
LL | #![deny(unused)]
11+
| ^^^^^^
12+
= note: `#[deny(unused_variables)]` implied by `#[deny(unused)]`
13+
help: try ignoring the field
14+
|
15+
LL | A { i, j: _ } | B { i, j: _ } => {
16+
| ^^^^ ^^^^
17+
18+
error: unused variable: `j`
19+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:31:16
20+
|
21+
LL | A { i, ref j } | B { i, ref j } => {
22+
| ^^^^^ ^^^^^
23+
|
24+
help: try ignoring the field
25+
|
26+
LL | A { i, j: _ } | B { i, j: _ } => {
27+
| ^^^^ ^^^^
28+
29+
error: unused variable: `j`
30+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:41:21
31+
|
32+
LL | Some(A { i, j } | B { i, j }) => {
33+
| ^ ^
34+
|
35+
help: try ignoring the field
36+
|
37+
LL | Some(A { i, j: _ } | B { i, j: _ }) => {
38+
| ^^^^ ^^^^
39+
40+
error: unused variable: `j`
41+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:53:21
42+
|
43+
LL | Some(A { i, ref j } | B { i, ref j }) => {
44+
| ^^^^^ ^^^^^
45+
|
46+
help: try ignoring the field
47+
|
48+
LL | Some(A { i, j: _ } | B { i, j: _ }) => {
49+
| ^^^^ ^^^^
50+
51+
error: unused variable: `i`
52+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:63: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(_) => {
60+
| ^^^^ ^
61+
62+
error: unused variable: `i`
63+
--> $DIR/issue-67691-unused-field-in-or-pattern.rs:71: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(_) => {
71+
| ^^^^ ^
72+
73+
error: aborting due to 6 previous errors
74+

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

Lines changed: 4 additions & 4 deletions
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

0 commit comments

Comments
 (0)