in which the unused shorthand field pattern debacle/saga continues

In e4b1a79 (#47922), we corrected erroneous suggestions for unused
shorthand field pattern bindings, suggesting `field: _` where the
previous suggestion of `_field` wouldn't even have compiled
(#47390). Soon, it was revealed that this was insufficient (#50303), and
the fix was extended to references, slices, &c. (#50327) But even this
proved inadequate, as the erroneous suggestions were still being issued
for patterns in local (`let`) bindings (#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 #50804.

Author: zackmdavis

src/librustc/middle/liveness.rs

@@ -117,6 +117,7 @@ use std::io::prelude::*;
 use std::io;
 use std::rc::Rc;
 use syntax::ast::{self, NodeId};
+use syntax::ptr::P;
 use syntax::symbol::keywords;
 use syntax_pos::Span;
 
@@ -398,72 +399,65 @@ fn visit_fn<'a, 'tcx: 'a>(ir: &mut IrMaps<'a, 'tcx>,
     lsets.warn_about_unused_args(body, entry_ln);
 }
 
-fn visit_local<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, local: &'tcx hir::Local) {
-    local.pat.each_binding(|_, p_id, sp, path1| {
-        debug!("adding local variable {}", p_id);
+fn add_from_pat<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, pat: &P<hir::Pat>) {
+    // For struct patterns, take note of which fields used shorthand
+    // (`x` rather than `x: x`).
+    //
+    // FIXME: according to the rust-lang-nursery/rustc-guide book, `NodeId`s are to be
+    // phased out in favor of `HirId`s; however, we need to match the signature of
+    // `each_binding`, which uses `NodeIds`.
+    let mut shorthand_field_ids = NodeSet();
+    let mut pats = VecDeque::new();
+    pats.push_back(pat);
+    while let Some(pat) = pats.pop_front() {
+        use hir::PatKind::*;
+        match pat.node {
+            Binding(_, _, _, ref inner_pat) => {
+                pats.extend(inner_pat.iter());
+            }
+            Struct(_, ref fields, _) => {
+                for field in fields {
+                    if field.node.is_shorthand {
+                        shorthand_field_ids.insert(field.node.pat.id);
+                    }
+                }
+            }
+            Ref(ref inner_pat, _) |
+            Box(ref inner_pat) => {
+                pats.push_back(inner_pat);
+            }
+            TupleStruct(_, ref inner_pats, _) |
+            Tuple(ref inner_pats, _) => {
+                pats.extend(inner_pats.iter());
+            }
+            Slice(ref pre_pats, ref inner_pat, ref post_pats) => {
+                pats.extend(pre_pats.iter());
+                pats.extend(inner_pat.iter());
+                pats.extend(post_pats.iter());
+            }
+            _ => {}
+        }
+    }
+
+    pat.each_binding(|_bm, p_id, _sp, path1| {
         let name = path1.node;
-        ir.add_live_node_for_node(p_id, VarDefNode(sp));
+        ir.add_live_node_for_node(p_id, VarDefNode(path1.span));
         ir.add_variable(Local(LocalInfo {
             id: p_id,
             name,
-            is_shorthand: false,
+            is_shorthand: shorthand_field_ids.contains(&p_id)
         }));
     });
+}
+
+fn visit_local<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, local: &'tcx hir::Local) {
+    add_from_pat(ir, &local.pat);
     intravisit::walk_local(ir, local);
 }
 
 fn visit_arm<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, arm: &'tcx hir::Arm) {
-    for mut pat in &arm.pats {
-        // For struct patterns, take note of which fields used shorthand
-        // (`x` rather than `x: x`).
-        //
-        // FIXME: according to the rust-lang-nursery/rustc-guide book, `NodeId`s are to be
-        // phased out in favor of `HirId`s; however, we need to match the signature of
-        // `each_binding`, which uses `NodeIds`.
-        let mut shorthand_field_ids = NodeSet();
-        let mut pats = VecDeque::new();
-        pats.push_back(pat);
-        while let Some(pat) = pats.pop_front() {
-            use hir::PatKind::*;
-            match pat.node {
-                Binding(_, _, _, ref inner_pat) => {
-                    pats.extend(inner_pat.iter());
-                }
-                Struct(_, ref fields, _) => {
-                    for field in fields {
-                        if field.node.is_shorthand {
-                            shorthand_field_ids.insert(field.node.pat.id);
-                        }
-                    }
-                }
-                Ref(ref inner_pat, _) |
-                Box(ref inner_pat) => {
-                    pats.push_back(inner_pat);
-                }
-                TupleStruct(_, ref inner_pats, _) |
-                Tuple(ref inner_pats, _) => {
-                    pats.extend(inner_pats.iter());
-                }
-                Slice(ref pre_pats, ref inner_pat, ref post_pats) => {
-                    pats.extend(pre_pats.iter());
-                    pats.extend(inner_pat.iter());
-                    pats.extend(post_pats.iter());
-                }
-                _ => {}
-            }
-        }
-
-        pat.each_binding(|bm, p_id, _sp, path1| {
-            debug!("adding local variable {} from match with bm {:?}",
-                   p_id, bm);
-            let name = path1.node;
-            ir.add_live_node_for_node(p_id, VarDefNode(path1.span));
-            ir.add_variable(Local(LocalInfo {
-                id: p_id,
-                name: name,
-                is_shorthand: shorthand_field_ids.contains(&p_id)
-            }));
-        })
+    for pat in &arm.pats {
+        add_from_pat(ir, pat);
     }
     intravisit::walk_arm(ir, arm);
 }

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

@@ -20,6 +20,11 @@ struct SoulHistory {
     endless_and_singing: bool
 }
 
+struct LovelyAmbition {
+    lips: usize,
+    fire: usize
+}
+
 #[derive(Clone, Copy)]
 enum Large {
     Suit { case: () }
@@ -45,6 +50,10 @@ fn main() {
         hours_are_suns = false;
     }
 
+    let the_spirit = LovelyAmbition { lips: 1, fire: 2 };
+    let LovelyAmbition { lips, fire } = the_spirit;
+    println!("{}", lips);
+
     let bag = Large::Suit {
         case: ()
     };

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

@@ -1,5 +1,5 @@
 warning: unused variable: `i_think_continually`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:31:9
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:36:9
    |
 LL |     let i_think_continually = 2;
    |         ^^^^^^^^^^^^^^^^^^^ help: consider using `_i_think_continually` instead
@@ -12,39 +12,39 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
    = note: #[warn(unused_variables)] implied by #[warn(unused)]
 
 warning: unused variable: `mut_unused_var`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:38:13
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:43:13
    |
 LL |     let mut mut_unused_var = 1;
    |             ^^^^^^^^^^^^^^ help: consider using `_mut_unused_var` instead
 
 warning: unused variable: `var`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:14
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:14
    |
 LL |     let (mut var, unused_var) = (1, 2);
    |              ^^^ help: consider using `_var` instead
 
 warning: unused variable: `unused_var`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:19
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:19
    |
 LL |     let (mut var, unused_var) = (1, 2);
    |                   ^^^^^^^^^^ help: consider using `_unused_var` instead
 
 warning: unused variable: `corridors_of_light`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:42:26
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:47:26
    |
 LL |     if let SoulHistory { corridors_of_light,
    |                          ^^^^^^^^^^^^^^^^^^ help: try ignoring the field: `corridors_of_light: _`
 
 warning: variable `hours_are_suns` is assigned to, but never used
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:43:30
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:48:30
    |
 LL |                          mut hours_are_suns,
    |                              ^^^^^^^^^^^^^^
    |
    = note: consider using `_hours_are_suns` instead
 
 warning: value assigned to `hours_are_suns` is never read
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:9
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:50:9
    |
 LL |         hours_are_suns = false;
    |         ^^^^^^^^^^^^^^
@@ -56,44 +56,50 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
    |         ^^^^^^
    = note: #[warn(unused_assignments)] implied by #[warn(unused)]
 
+warning: unused variable: `fire`
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:54:32
+   |
+LL |     let LovelyAmbition { lips, fire } = the_spirit;
+   |                                ^^^^ help: try ignoring the field: `fire: _`
+
 warning: unused variable: `case`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:54:23
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:63:23
    |
 LL |         Large::Suit { case } => {}
    |                       ^^^^ help: try ignoring the field: `case: _`
 
 warning: unused variable: `case`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:59:24
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:68:24
    |
 LL |         &Large::Suit { case } => {}
    |                        ^^^^ help: try ignoring the field: `case: _`
 
 warning: unused variable: `case`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:64:27
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:73:27
    |
 LL |         box Large::Suit { case } => {}
    |                           ^^^^ help: try ignoring the field: `case: _`
 
 warning: unused variable: `case`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:69:24
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:78:24
    |
 LL |         (Large::Suit { case },) => {}
    |                        ^^^^ help: try ignoring the field: `case: _`
 
 warning: unused variable: `case`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:74:24
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:83:24
    |
 LL |         [Large::Suit { case }] => {}
    |                        ^^^^ help: try ignoring the field: `case: _`
 
 warning: unused variable: `case`
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:79:29
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:88:29
    |
 LL |         Tuple(Large::Suit { case }, ()) => {}
    |                             ^^^^ help: try ignoring the field: `case: _`
 
 warning: variable does not need to be mutable
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:38:9
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:43:9
    |
 LL |     let mut mut_unused_var = 1;
    |         ----^^^^^^^^^^^^^^
@@ -108,7 +114,7 @@ LL | #![warn(unused)] // UI tests pass `-A unused` (#43896)
    = note: #[warn(unused_mut)] implied by #[warn(unused)]
 
 warning: variable does not need to be mutable
-  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:40:10
+  --> $DIR/issue-47390-unused-variable-in-struct-pattern.rs:45:10
    |
 LL |     let (mut var, unused_var) = (1, 2);
    |          ----^^^