Skip to content

Commit 907288c

Browse files
kennytmkennytm
committed
Rollup merge of rust-lang#50854 - zackmdavis:and_the_case_of_the_unused_field_pattern_3_straight_to_video, r=estebank
in which the unused shorthand field pattern debacle/saga continues In e4b1a79 (rust-lang#47922), we corrected erroneous suggestions for unused shorthand field pattern bindings, suggesting `field: _` where the previous suggestion of `_field` wouldn't even have compiled (rust-lang#47390). Soon, it was revealed that this was insufficient (rust-lang#50303), and the fix was extended to references, slices, &c. (rust-lang#50327) But even this proved inadequate, as the erroneous suggestions were still being issued for patterns in local (`let`) bindings (rust-lang#50804). Here, we yank the shorthand-detection and variable/node registration code into a new common function that can be called while visiting both match arms and `let` bindings. Resolves rust-lang#50804. r? @estebank
2 parents 611dafc + 59782f4 commit 907288c

File tree

3 files changed

+80
-71
lines changed

3 files changed

+80
-71
lines changed

‎src/librustc/middle/liveness.rs

+50-56
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ use std::io::prelude::*;
117117
use std::io;
118118
use std::rc::Rc;
119119
use syntax::ast::{self, NodeId};
120+
use syntax::ptr::P;
120121
use syntax::symbol::keywords;
121122
use syntax_pos::Span;
122123

@@ -398,72 +399,65 @@ fn visit_fn<'a, 'tcx: 'a>(ir: &mut IrMaps<'a, 'tcx>,
398399
lsets.warn_about_unused_args(body, entry_ln);
399400
}
400401

401-
fn visit_local<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, local: &'tcx hir::Local) {
402-
local.pat.each_binding(|_, p_id, sp, path1| {
403-
debug!("adding local variable {}", p_id);
402+
fn add_from_pat<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, pat: &P<hir::Pat>) {
403+
// For struct patterns, take note of which fields used shorthand
404+
// (`x` rather than `x: x`).
405+
//
406+
// FIXME: according to the rust-lang-nursery/rustc-guide book, `NodeId`s are to be
407+
// phased out in favor of `HirId`s; however, we need to match the signature of
408+
// `each_binding`, which uses `NodeIds`.
409+
let mut shorthand_field_ids = NodeSet();
410+
let mut pats = VecDeque::new();
411+
pats.push_back(pat);
412+
while let Some(pat) = pats.pop_front() {
413+
use hir::PatKind::*;
414+
match pat.node {
415+
Binding(_, _, _, ref inner_pat) => {
416+
pats.extend(inner_pat.iter());
417+
}
418+
Struct(_, ref fields, _) => {
419+
for field in fields {
420+
if field.node.is_shorthand {
421+
shorthand_field_ids.insert(field.node.pat.id);
422+
}
423+
}
424+
}
425+
Ref(ref inner_pat, _) |
426+
Box(ref inner_pat) => {
427+
pats.push_back(inner_pat);
428+
}
429+
TupleStruct(_, ref inner_pats, _) |
430+
Tuple(ref inner_pats, _) => {
431+
pats.extend(inner_pats.iter());
432+
}
433+
Slice(ref pre_pats, ref inner_pat, ref post_pats) => {
434+
pats.extend(pre_pats.iter());
435+
pats.extend(inner_pat.iter());
436+
pats.extend(post_pats.iter());
437+
}
438+
_ => {}
439+
}
440+
}
441+
442+
pat.each_binding(|_bm, p_id, _sp, path1| {
404443
let name = path1.node;
405-
ir.add_live_node_for_node(p_id, VarDefNode(sp));
444+
ir.add_live_node_for_node(p_id, VarDefNode(path1.span));
406445
ir.add_variable(Local(LocalInfo {
407446
id: p_id,
408447
name,
409-
is_shorthand: false,
448+
is_shorthand: shorthand_field_ids.contains(&p_id)
410449
}));
411450
});
451+
}
452+
453+
fn visit_local<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, local: &'tcx hir::Local) {
454+
add_from_pat(ir, &local.pat);
412455
intravisit::walk_local(ir, local);
413456
}
414457

415458
fn visit_arm<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, arm: &'tcx hir::Arm) {
416-
for mut pat in &arm.pats {
417-
// For struct patterns, take note of which fields used shorthand
418-
// (`x` rather than `x: x`).
419-
//
420-
// FIXME: according to the rust-lang-nursery/rustc-guide book, `NodeId`s are to be
421-
// phased out in favor of `HirId`s; however, we need to match the signature of
422-
// `each_binding`, which uses `NodeIds`.
423-
let mut shorthand_field_ids = NodeSet();
424-
let mut pats = VecDeque::new();
425-
pats.push_back(pat);
426-
while let Some(pat) = pats.pop_front() {
427-
use hir::PatKind::*;
428-
match pat.node {
429-
Binding(_, _, _, ref inner_pat) => {
430-
pats.extend(inner_pat.iter());
431-
}
432-
Struct(_, ref fields, _) => {
433-
for field in fields {
434-
if field.node.is_shorthand {
435-
shorthand_field_ids.insert(field.node.pat.id);
436-
}
437-
}
438-
}
439-
Ref(ref inner_pat, _) |
440-
Box(ref inner_pat) => {
441-
pats.push_back(inner_pat);
442-
}
443-
TupleStruct(_, ref inner_pats, _) |
444-
Tuple(ref inner_pats, _) => {
445-
pats.extend(inner_pats.iter());
446-
}
447-
Slice(ref pre_pats, ref inner_pat, ref post_pats) => {
448-
pats.extend(pre_pats.iter());
449-
pats.extend(inner_pat.iter());
450-
pats.extend(post_pats.iter());
451-
}
452-
_ => {}
453-
}
454-
}
455-
456-
pat.each_binding(|bm, p_id, _sp, path1| {
457-
debug!("adding local variable {} from match with bm {:?}",
458-
p_id, bm);
459-
let name = path1.node;
460-
ir.add_live_node_for_node(p_id, VarDefNode(path1.span));
461-
ir.add_variable(Local(LocalInfo {
462-
id: p_id,
463-
name: name,
464-
is_shorthand: shorthand_field_ids.contains(&p_id)
465-
}));
466-
})
459+
for pat in &arm.pats {
460+
add_from_pat(ir, pat);
467461
}
468462
intravisit::walk_arm(ir, arm);
469463
}

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

+9
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ struct SoulHistory {
2020
endless_and_singing: bool
2121
}
2222

23+
struct LovelyAmbition {
24+
lips: usize,
25+
fire: usize
26+
}
27+
2328
#[derive(Clone, Copy)]
2429
enum Large {
2530
Suit { case: () }
@@ -45,6 +50,10 @@ fn main() {
4550
hours_are_suns = false;
4651
}
4752

53+
let the_spirit = LovelyAmbition { lips: 1, fire: 2 };
54+
let LovelyAmbition { lips, fire } = the_spirit;
55+
println!("{}", lips);
56+
4857
let bag = Large::Suit {
4958
case: ()
5059
};

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

+21-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: unused variable: `i_think_continually`
2-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:31:9
2+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:36:9
33
|
44
LL | let i_think_continually = 2;
55
| ^^^^^^^^^^^^^^^^^^^ help: consider using `_i_think_continually` instead
@@ -12,39 +12,39 @@ 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:38:13
15+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:43:13
1616
|
1717
LL | let mut mut_unused_var = 1;
1818
| ^^^^^^^^^^^^^^ help: consider using `_mut_unused_var` instead
1919

2020
warning: unused variable: `var`
21-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:14
21+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:14
2222
|
2323
LL | let (mut var, unused_var) = (1, 2);
2424
| ^^^ help: consider using `_var` instead
2525

2626
warning: unused variable: `unused_var`
27-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:19
27+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:19
2828
|
2929
LL | let (mut var, unused_var) = (1, 2);
3030
| ^^^^^^^^^^ help: consider using `_unused_var` instead
3131

3232
warning: unused variable: `corridors_of_light`
33-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:42:26
33+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:47:26
3434
|
3535
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:43:30
39+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:48:30
4040
|
4141
LL | mut hours_are_suns,
4242
| ^^^^^^^^^^^^^^
4343
|
4444
= note: consider using `_hours_are_suns` instead
4545

4646
warning: value assigned to `hours_are_suns` is never read
47-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:9
47+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:50:9
4848
|
4949
LL | hours_are_suns = false;
5050
| ^^^^^^^^^^^^^^
@@ -56,44 +56,50 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
5656
| ^^^^^^
5757
= note: #[warn(unused_assignments)] implied by #[warn(unused)]
5858

59+
warning: unused variable: `fire`
60+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:54:32
61+
|
62+
LL | let LovelyAmbition { lips, fire } = the_spirit;
63+
| ^^^^ help: try ignoring the field: `fire: _`
64+
5965
warning: unused variable: `case`
60-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:54:23
66+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:63:23
6167
|
6268
LL | Large::Suit { case } => {}
6369
| ^^^^ help: try ignoring the field: `case: _`
6470

6571
warning: unused variable: `case`
66-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:59:24
72+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:68:24
6773
|
6874
LL | &Large::Suit { case } => {}
6975
| ^^^^ help: try ignoring the field: `case: _`
7076

7177
warning: unused variable: `case`
72-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:64:27
78+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:73:27
7379
|
7480
LL | box Large::Suit { case } => {}
7581
| ^^^^ help: try ignoring the field: `case: _`
7682

7783
warning: unused variable: `case`
78-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:69:24
84+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:78:24
7985
|
8086
LL | (Large::Suit { case },) => {}
8187
| ^^^^ help: try ignoring the field: `case: _`
8288

8389
warning: unused variable: `case`
84-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:74:24
90+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:83:24
8591
|
8692
LL | [Large::Suit { case }] => {}
8793
| ^^^^ help: try ignoring the field: `case: _`
8894

8995
warning: unused variable: `case`
90-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:79:29
96+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:88:29
9197
|
9298
LL | Tuple(Large::Suit { case }, ()) => {}
9399
| ^^^^ help: try ignoring the field: `case: _`
94100

95101
warning: variable does not need to be mutable
96-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:38:9
102+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:43:9
97103
|
98104
LL | let mut mut_unused_var = 1;
99105
| ----^^^^^^^^^^^^^^
@@ -108,7 +114,7 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
108114
= note: #[warn(unused_mut)] implied by #[warn(unused)]
109115

110116
warning: variable does not need to be mutable
111-
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:10
117+
--> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:10
112118
|
113119
LL | let (mut var, unused_var) = (1, 2);
114120
| ----^^^

0 commit comments

Comments
 (0)