Skip to content

Commit 93350f4

Browse files
CentrilCentril
authored
Rollup merge of rust-lang#57899 - davidtwco:issue-56685, r=estebank
Unused variable suggestions apply on all patterns. Fixes rust-lang#56685. This PR extends existing suggestions to prefix unused variable bindings in match arms with an underscore so that it applies to all patterns in a match arm. r? @estebank cc @alexcrichton (since you filed the issue)
2 parents 20c2cba + 12cdaec commit 93350f4

17 files changed

+181
-56
lines changed

‎src/librustc/middle/liveness.rs

+52-31
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ use ty::query::{Providers, queries};
104104
use lint;
105105
use errors::Applicability;
106106
use util::nodemap::{NodeMap, HirIdMap, HirIdSet};
107+
use rustc_data_structures::fx::FxHashMap;
107108

108109
use std::collections::VecDeque;
109110
use std::{fmt, u32};
@@ -1446,7 +1447,7 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local)
14461447
None => {
14471448
this.pat_bindings(&local.pat, |this, ln, var, sp, id| {
14481449
let span = local.pat.simple_ident().map_or(sp, |ident| ident.span);
1449-
this.warn_about_unused(span, id, ln, var);
1450+
this.warn_about_unused(vec![span], id, ln, var);
14501451
})
14511452
}
14521453
}
@@ -1455,12 +1456,29 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local)
14551456
}
14561457

14571458
fn check_arm<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, arm: &'tcx hir::Arm) {
1458-
// only consider the first pattern; any later patterns must have
1459-
// the same bindings, and we also consider the first pattern to be
1460-
// the "authoritative" set of ids
1461-
this.arm_pats_bindings(arm.pats.first().map(|p| &**p), |this, ln, var, sp, id| {
1462-
this.warn_about_unused(sp, id, ln, var);
1463-
});
1459+
// Only consider the variable from the first pattern; any later patterns must have
1460+
// the same bindings, and we also consider the first pattern to be the "authoritative" set of
1461+
// ids. However, we should take the spans of variables with the same name from the later
1462+
// patterns so the suggestions to prefix with underscores will apply to those too.
1463+
let mut vars: FxHashMap<String, (LiveNode, Variable, HirId, Vec<Span>)> = Default::default();
1464+
1465+
for pat in &arm.pats {
1466+
this.arm_pats_bindings(Some(&*pat), |this, ln, var, sp, id| {
1467+
let name = this.ir.variable_name(var);
1468+
vars.entry(name)
1469+
.and_modify(|(.., spans)| {
1470+
spans.push(sp);
1471+
})
1472+
.or_insert_with(|| {
1473+
(ln, var, id, vec![sp])
1474+
});
1475+
});
1476+
}
1477+
1478+
for (_, (ln, var, id, spans)) in vars {
1479+
this.warn_about_unused(spans, id, ln, var);
1480+
}
1481+
14641482
intravisit::walk_arm(this, arm);
14651483
}
14661484

@@ -1551,7 +1569,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15511569
let var = self.variable(hir_id, sp);
15521570
// Ignore unused self.
15531571
if ident.name != keywords::SelfLower.name() {
1554-
if !self.warn_about_unused(sp, hir_id, entry_ln, var) {
1572+
if !self.warn_about_unused(vec![sp], hir_id, entry_ln, var) {
15551573
if self.live_on_entry(entry_ln, var).is_none() {
15561574
self.report_dead_assign(hir_id, sp, var, true);
15571575
}
@@ -1563,14 +1581,14 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15631581

15641582
fn warn_about_unused_or_dead_vars_in_pat(&mut self, pat: &hir::Pat) {
15651583
self.pat_bindings(pat, |this, ln, var, sp, id| {
1566-
if !this.warn_about_unused(sp, id, ln, var) {
1584+
if !this.warn_about_unused(vec![sp], id, ln, var) {
15671585
this.warn_about_dead_assign(sp, id, ln, var);
15681586
}
15691587
})
15701588
}
15711589

15721590
fn warn_about_unused(&self,
1573-
sp: Span,
1591+
spans: Vec<Span>,
15741592
hir_id: HirId,
15751593
ln: LiveNode,
15761594
var: Variable)
@@ -1587,29 +1605,36 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15871605
self.assigned_on_exit(ln, var).is_some()
15881606
};
15891607

1590-
let suggest_underscore_msg = format!("consider using `_{}` instead", name);
1591-
15921608
if is_assigned {
1593-
self.ir.tcx
1594-
.lint_hir_note(lint::builtin::UNUSED_VARIABLES, hir_id, sp,
1595-
&format!("variable `{}` is assigned to, but never used",
1596-
name),
1597-
&suggest_underscore_msg);
1609+
self.ir.tcx.lint_hir_note(
1610+
lint::builtin::UNUSED_VARIABLES,
1611+
hir_id,
1612+
spans.clone(),
1613+
&format!("variable `{}` is assigned to, but never used", name),
1614+
&format!("consider using `_{}` instead", name),
1615+
);
15981616
} else if name != "self" {
1599-
let msg = format!("unused variable: `{}`", name);
1600-
let mut err = self.ir.tcx
1601-
.struct_span_lint_hir(lint::builtin::UNUSED_VARIABLES, hir_id, sp, &msg);
1617+
let mut err = self.ir.tcx.struct_span_lint_hir(
1618+
lint::builtin::UNUSED_VARIABLES,
1619+
hir_id,
1620+
spans.clone(),
1621+
&format!("unused variable: `{}`", name),
1622+
);
1623+
16021624
if self.ir.variable_is_shorthand(var) {
1603-
err.span_suggestion_with_applicability(sp, "try ignoring the field",
1604-
format!("{}: _", name),
1605-
Applicability::MachineApplicable);
1625+
err.multipart_suggestion_with_applicability(
1626+
"try ignoring the field",
1627+
spans.iter().map(|span| (*span, format!("{}: _", name))).collect(),
1628+
Applicability::MachineApplicable
1629+
);
16061630
} else {
1607-
err.span_suggestion_short_with_applicability(
1608-
sp, &suggest_underscore_msg,
1609-
format!("_{}", name),
1631+
err.multipart_suggestion_with_applicability(
1632+
"consider prefixing with an underscore",
1633+
spans.iter().map(|span| (*span, format!("_{}", name))).collect(),
16101634
Applicability::MachineApplicable,
16111635
);
16121636
}
1637+
16131638
err.emit()
16141639
}
16151640
}
@@ -1619,11 +1644,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
16191644
}
16201645
}
16211646

1622-
fn warn_about_dead_assign(&self,
1623-
sp: Span,
1624-
hir_id: HirId,
1625-
ln: LiveNode,
1626-
var: Variable) {
1647+
fn warn_about_dead_assign(&self, sp: Span, hir_id: HirId, ln: LiveNode, var: Variable) {
16271648
if self.live_on_exit(ln, var).is_none() {
16281649
self.report_dead_assign(hir_id, sp, var, false);
16291650
}

‎src/test/ui/issues/issue-17999.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `x`
22
--> $DIR/issue-17999.rs:5:13
33
|
44
LL | let x = (); //~ ERROR: unused variable: `x`
5-
| ^ help: consider using `_x` instead
5+
| ^ help: consider prefixing with an underscore: `_x`
66
|
77
note: lint level defined here
88
--> $DIR/issue-17999.rs:1:9
@@ -14,7 +14,7 @@ error: unused variable: `a`
1414
--> $DIR/issue-17999.rs:7:13
1515
|
1616
LL | a => {} //~ ERROR: unused variable: `a`
17-
| ^ help: consider using `_a` instead
17+
| ^ help: consider prefixing with an underscore: `_a`
1818

1919
error: aborting due to 2 previous errors
2020

‎src/test/ui/issues/issue-22599.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `a`
22
--> $DIR/issue-22599.rs:8:19
33
|
44
LL | v = match 0 { a => 0 }; //~ ERROR: unused variable: `a`
5-
| ^ help: consider using `_a` instead
5+
| ^ help: consider prefixing with an underscore: `_a`
66
|
77
note: lint level defined here
88
--> $DIR/issue-22599.rs:1:9

‎src/test/ui/issues/issue-56685.rs

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#![allow(dead_code)]
2+
#![deny(unused_variables)]
3+
4+
// This test aims to check that unused variable suggestions update bindings in all
5+
// match arms.
6+
7+
fn main() {
8+
enum E {
9+
A(i32,),
10+
B(i32,),
11+
}
12+
13+
match E::A(1) {
14+
E::A(x) | E::B(x) => {}
15+
//~^ ERROR unused variable: `x`
16+
}
17+
18+
enum F {
19+
A(i32, i32,),
20+
B(i32, i32,),
21+
C(i32, i32,),
22+
}
23+
24+
let _ = match F::A(1, 2) {
25+
F::A(x, y) | F::B(x, y) => { y },
26+
//~^ ERROR unused variable: `x`
27+
F::C(a, b) => { 3 }
28+
//~^ ERROR unused variable: `a`
29+
//~^^ ERROR unused variable: `b`
30+
};
31+
32+
let _ = if let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
33+
//~^ ERROR unused variable: `x`
34+
y
35+
} else {
36+
3
37+
};
38+
39+
while let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
40+
//~^ ERROR unused variable: `x`
41+
let _ = y;
42+
break;
43+
}
44+
}

‎src/test/ui/issues/issue-56685.stderr

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
error: unused variable: `x`
2+
--> $DIR/issue-56685.rs:14:14
3+
|
4+
LL | E::A(x) | E::B(x) => {}
5+
| ^ ^
6+
|
7+
note: lint level defined here
8+
--> $DIR/issue-56685.rs:2:9
9+
|
10+
LL | #![deny(unused_variables)]
11+
| ^^^^^^^^^^^^^^^^
12+
help: consider prefixing with an underscore
13+
|
14+
LL | E::A(_x) | E::B(_x) => {}
15+
| ^^ ^^
16+
17+
error: unused variable: `x`
18+
--> $DIR/issue-56685.rs:25:14
19+
|
20+
LL | F::A(x, y) | F::B(x, y) => { y },
21+
| ^ ^
22+
help: consider prefixing with an underscore
23+
|
24+
LL | F::A(_x, y) | F::B(_x, y) => { y },
25+
| ^^ ^^
26+
27+
error: unused variable: `b`
28+
--> $DIR/issue-56685.rs:27:17
29+
|
30+
LL | F::C(a, b) => { 3 }
31+
| ^ help: consider prefixing with an underscore: `_b`
32+
33+
error: unused variable: `a`
34+
--> $DIR/issue-56685.rs:27:14
35+
|
36+
LL | F::C(a, b) => { 3 }
37+
| ^ help: consider prefixing with an underscore: `_a`
38+
39+
error: unused variable: `x`
40+
--> $DIR/issue-56685.rs:32:25
41+
|
42+
LL | let _ = if let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
43+
| ^ ^
44+
help: consider prefixing with an underscore
45+
|
46+
LL | let _ = if let F::A(_x, y) | F::B(_x, y) = F::A(1, 2) {
47+
| ^^ ^^
48+
49+
error: unused variable: `x`
50+
--> $DIR/issue-56685.rs:39:20
51+
|
52+
LL | while let F::A(x, y) | F::B(x, y) = F::A(1, 2) {
53+
| ^ ^
54+
help: consider prefixing with an underscore
55+
|
56+
LL | while let F::A(_x, y) | F::B(_x, y) = F::A(1, 2) {
57+
| ^^ ^^
58+
59+
error: aborting due to 6 previous errors
60+

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ warning: unused variable: `i_think_continually`
22
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:26:9
33
|
44
LL | let i_think_continually = 2;
5-
| ^^^^^^^^^^^^^^^^^^^ help: consider using `_i_think_continually` instead
5+
| ^^^^^^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_i_think_continually`
66
|
77
note: lint level defined here
88
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:5:9
@@ -15,19 +15,19 @@ warning: unused variable: `mut_unused_var`
1515
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:33:13
1616
|
1717
LL | let mut mut_unused_var = 1;
18-
| ^^^^^^^^^^^^^^ help: consider using `_mut_unused_var` instead
18+
| ^^^^^^^^^^^^^^ help: consider prefixing with an underscore: `_mut_unused_var`
1919

2020
warning: unused variable: `var`
2121
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:35:14
2222
|
2323
LL | let (mut var, unused_var) = (1, 2);
24-
| ^^^ help: consider using `_var` instead
24+
| ^^^ help: consider prefixing with an underscore: `_var`
2525

2626
warning: unused variable: `unused_var`
2727
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:35:19
2828
|
2929
LL | let (mut var, unused_var) = (1, 2);
30-
| ^^^^^^^^^^ help: consider using `_unused_var` instead
30+
| ^^^^^^^^^^ help: consider prefixing with an underscore: `_unused_var`
3131

3232
warning: unused variable: `corridors_of_light`
3333
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:37:26

‎src/test/ui/lint/lint-removed-allow.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `unused`
22
--> $DIR/lint-removed-allow.rs:8:17
33
|
44
LL | fn main() { let unused = (); } //~ ERROR unused
5-
| ^^^^^^ help: consider using `_unused` instead
5+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
66
|
77
note: lint level defined here
88
--> $DIR/lint-removed-allow.rs:7:8

‎src/test/ui/lint/lint-removed-cmdline.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ error: unused variable: `unused`
66
--> $DIR/lint-removed-cmdline.rs:12:17
77
|
88
LL | fn main() { let unused = (); }
9-
| ^^^^^^ help: consider using `_unused` instead
9+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
1010
|
1111
note: lint level defined here
1212
--> $DIR/lint-removed-cmdline.rs:11:8

‎src/test/ui/lint/lint-removed.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ error: unused variable: `unused`
1010
--> $DIR/lint-removed.rs:8:17
1111
|
1212
LL | fn main() { let unused = (); } //~ ERROR unused
13-
| ^^^^^^ help: consider using `_unused` instead
13+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
1414
|
1515
note: lint level defined here
1616
--> $DIR/lint-removed.rs:7:8

‎src/test/ui/lint/lint-renamed-allow.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: unused variable: `unused`
22
--> $DIR/lint-renamed-allow.rs:8:17
33
|
44
LL | fn main() { let unused = (); } //~ ERROR unused
5-
| ^^^^^^ help: consider using `_unused` instead
5+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
66
|
77
note: lint level defined here
88
--> $DIR/lint-renamed-allow.rs:7:8

‎src/test/ui/lint/lint-renamed-cmdline.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ error: unused variable: `unused`
66
--> $DIR/lint-renamed-cmdline.rs:8:17
77
|
88
LL | fn main() { let unused = (); }
9-
| ^^^^^^ help: consider using `_unused` instead
9+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
1010
|
1111
note: lint level defined here
1212
--> $DIR/lint-renamed-cmdline.rs:7:8

‎src/test/ui/lint/lint-renamed.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ error: unused variable: `unused`
1010
--> $DIR/lint-renamed.rs:4:17
1111
|
1212
LL | fn main() { let unused = (); } //~ ERROR unused
13-
| ^^^^^^ help: consider using `_unused` instead
13+
| ^^^^^^ help: consider prefixing with an underscore: `_unused`
1414
|
1515
note: lint level defined here
1616
--> $DIR/lint-renamed.rs:3:8

‎src/test/ui/lint/lint-uppercase-variables.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ warning: unused variable: `Foo`
88
--> $DIR/lint-uppercase-variables.rs:22:9
99
|
1010
LL | Foo => {}
11-
| ^^^ help: consider using `_Foo` instead
11+
| ^^^ help: consider prefixing with an underscore: `_Foo`
1212
|
1313
note: lint level defined here
1414
--> $DIR/lint-uppercase-variables.rs:1:9

0 commit comments

Comments
 (0)