Auto merge of #60174 - matthewjasper:add-match-arm-scopes, r=pnkfelix

Add match arm scopes and other scope fixes

* Add drop and lint scopes for match arms.
* Lint attributes are now respected on match arms.
* Make sure we emit a StorageDead if we diverge when initializing a temporary.
* Adjust MIR pretty printing of scopes for locals.
* Don't generate duplicate lint scopes for `let statements`.
* Add some previously missing fake borrows for matches.

closes #46525

cc @rust-lang/compiler

Author: bors

src/librustc/cfg/construct.rs

@@ -419,7 +419,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
         for arm in arms {
             // Add an exit node for when we've visited all the
             // patterns and the guard (if there is one) in the arm.
-            let arm_exit = self.add_dummy_node(&[]);
+            let bindings_exit = self.add_dummy_node(&[]);
 
             for pat in &arm.pats {
                 // Visit the pattern, coming from the discriminant exit
@@ -453,14 +453,16 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
 
                 // Add an edge from the exit of this pattern to the
                 // exit of the arm
-                self.add_contained_edge(pat_exit, arm_exit);
+                self.add_contained_edge(pat_exit, bindings_exit);
             }
 
             // Visit the body of this arm
-            let body_exit = self.expr(&arm.body, arm_exit);
+            let body_exit = self.expr(&arm.body, bindings_exit);
+
+            let arm_exit = self.add_ast_node(arm.hir_id.local_id, &[body_exit]);
 
             // Link the body to the exit of the expression
-            self.add_contained_edge(body_exit, expr_exit);
+            self.add_contained_edge(arm_exit, expr_exit);
         }
 
         expr_exit

src/librustc/hir/intravisit.rs

@@ -1102,6 +1102,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
 }
 
 pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
+    visitor.visit_id(arm.hir_id);
     walk_list!(visitor, visit_pat, &arm.pats);
     if let Some(ref g) = arm.guard {
         match g {

src/librustc/hir/lowering.rs

@@ -1314,13 +1314,15 @@ impl<'a> LoweringContext<'a> {
 
     fn lower_arm(&mut self, arm: &Arm) -> hir::Arm {
         hir::Arm {
+            hir_id: self.next_id(),
             attrs: self.lower_attrs(&arm.attrs),
             pats: arm.pats.iter().map(|x| self.lower_pat(x)).collect(),
             guard: match arm.guard {
                 Some(Guard::If(ref x)) => Some(hir::Guard::If(P(self.lower_expr(x)))),
                 _ => None,
             },
             body: P(self.lower_expr(&arm.body)),
+            span: arm.span,
         }
     }
 
@@ -5024,9 +5026,11 @@ impl<'a> LoweringContext<'a> {
 
     fn arm(&mut self, pats: hir::HirVec<P<hir::Pat>>, expr: P<hir::Expr>) -> hir::Arm {
         hir::Arm {
+            hir_id: self.next_id(),
             attrs: hir_vec![],
             pats,
             guard: None,
+            span: expr.span,
             body: expr,
         }
     }

src/librustc/hir/map/collector.rs

@@ -430,6 +430,16 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
         });
     }
 
+    fn visit_arm(&mut self, arm: &'hir Arm) {
+        let node = Node::Arm(arm);
+
+        self.insert(arm.span, arm.hir_id, node);
+
+        self.with_parent(arm.hir_id, |this| {
+            intravisit::walk_arm(this, arm);
+        });
+    }
+
     fn visit_anon_const(&mut self, constant: &'hir AnonConst) {
         self.insert(DUMMY_SP, constant.hir_id, Node::AnonConst(constant));
 

src/librustc/hir/map/mod.rs

@@ -373,6 +373,7 @@ impl<'hir> Map<'hir> {
             Node::Pat(_) |
             Node::Binding(_) |
             Node::Local(_) |
+            Node::Arm(_) |
             Node::Lifetime(_) |
             Node::Visibility(_) |
             Node::Block(_) |
@@ -1000,6 +1001,7 @@ impl<'hir> Map<'hir> {
             Some(Node::Field(ref f)) => Some(&f.attrs[..]),
             Some(Node::Expr(ref e)) => Some(&*e.attrs),
             Some(Node::Stmt(ref s)) => Some(s.node.attrs()),
+            Some(Node::Arm(ref a)) => Some(&*a.attrs),
             Some(Node::GenericParam(param)) => Some(&param.attrs[..]),
             // Unit/tuple structs/variants take the attributes straight from
             // the struct/variant definition.
@@ -1073,6 +1075,7 @@ impl<'hir> Map<'hir> {
             Some(Node::TraitRef(tr)) => tr.path.span,
             Some(Node::Binding(pat)) => pat.span,
             Some(Node::Pat(pat)) => pat.span,
+            Some(Node::Arm(arm)) => arm.span,
             Some(Node::Block(block)) => block.span,
             Some(Node::Ctor(..)) => match self.find_by_hir_id(
                 self.get_parent_node_by_hir_id(hir_id))
@@ -1288,6 +1291,7 @@ impl<'a> print::State<'a> {
             Node::TraitRef(a)     => self.print_trait_ref(&a),
             Node::Binding(a)      |
             Node::Pat(a)          => self.print_pat(&a),
+            Node::Arm(a)          => self.print_arm(&a),
             Node::Block(a)        => {
                 use syntax::print::pprust::PrintState;
 
@@ -1417,6 +1421,9 @@ fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String {
         Some(Node::Pat(_)) => {
             format!("pat {}{}", map.hir_to_pretty_string(id), id_str)
         }
+        Some(Node::Arm(_)) => {
+            format!("arm {}{}", map.hir_to_pretty_string(id), id_str)
+        }
         Some(Node::Block(_)) => {
             format!("block {}{}", map.hir_to_pretty_string(id), id_str)
         }

src/librustc/hir/mod.rs

@@ -1228,6 +1228,9 @@ pub struct Local {
 /// `<pats> (if <guard>) => <body>`.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
 pub struct Arm {
+    #[stable_hasher(ignore)]
+    pub hir_id: HirId,
+    pub span: Span,
     pub attrs: HirVec<Attribute>,
     /// Multiple patterns can be combined with `|`
     pub pats: HirVec<P<Pat>>,
@@ -2656,6 +2659,7 @@ pub enum Node<'hir> {
     TraitRef(&'hir TraitRef),
     Binding(&'hir Pat),
     Pat(&'hir Pat),
+    Arm(&'hir Arm),
     Block(&'hir Block),
     Local(&'hir Local),
     MacroDef(&'hir MacroDef),

src/librustc/hir/print.rs

@@ -1862,7 +1862,7 @@ impl<'a> State<'a> {
         self.ann.post(self, AnnNode::Pat(pat))
     }
 
-    fn print_arm(&mut self, arm: &hir::Arm) -> io::Result<()> {
+    pub fn print_arm(&mut self, arm: &hir::Arm) -> io::Result<()> {
         // I have no idea why this check is necessary, but here it
         // is :(
         if arm.attrs.is_empty() {

src/librustc/lint/mod.rs

@@ -852,6 +852,12 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for LintLevelMapBuilder<'a, 'tcx> {
         })
     }
 
+    fn visit_arm(&mut self, a: &'tcx hir::Arm) {
+        self.with_lint_attrs(a.hir_id, &a.attrs, |builder| {
+            intravisit::walk_arm(builder, a);
+        })
+    }
+
     fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) {
         self.with_lint_attrs(trait_item.hir_id, &trait_item.attrs, |builder| {
             intravisit::walk_trait_item(builder, trait_item);

src/librustc/middle/region.rs

@@ -119,18 +119,18 @@ impl fmt::Debug for Scope {
 pub enum ScopeData {
     Node,
 
-    // Scope of the call-site for a function or closure
-    // (outlives the arguments as well as the body).
+    /// Scope of the call-site for a function or closure
+    /// (outlives the arguments as well as the body).
     CallSite,
 
-    // Scope of arguments passed to a function or closure
-    // (they outlive its body).
+    /// Scope of arguments passed to a function or closure
+    /// (they outlive its body).
     Arguments,
 
-    // Scope of destructors for temporaries of node-id.
+    /// Scope of destructors for temporaries of node-id.
     Destruction,
 
-    // Scope following a `let id = expr;` binding in a block.
+    /// Scope following a `let id = expr;` binding in a block.
     Remainder(FirstStatementIndex)
 }
 
@@ -152,11 +152,11 @@ newtype_index! {
     ///
     /// * The subscope with `first_statement_index == 1` is scope of `c`,
     ///   and thus does not include EXPR_2, but covers the `...`.
-    pub struct FirstStatementIndex { .. }
+    pub struct FirstStatementIndex {
+        derive [HashStable]
+    }
 }
 
-impl_stable_hash_for!(struct crate::middle::region::FirstStatementIndex { private });
-
 // compilation error if size of `ScopeData` is not the same as a `u32`
 static_assert_size!(ScopeData, 4);
 
@@ -814,13 +814,25 @@ fn resolve_block<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, blk:
 }
 
 fn resolve_arm<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, arm: &'tcx hir::Arm) {
+    let prev_cx = visitor.cx;
+
+    visitor.enter_scope(
+        Scope {
+            id: arm.hir_id.local_id,
+            data: ScopeData::Node,
+        }
+    );
+    visitor.cx.var_parent = visitor.cx.parent;
+
     visitor.terminating_scopes.insert(arm.body.hir_id.local_id);
 
     if let Some(hir::Guard::If(ref expr)) = arm.guard {
         visitor.terminating_scopes.insert(expr.hir_id.local_id);
     }
 
     intravisit::walk_arm(visitor, arm);
+
+    visitor.cx = prev_cx;
 }
 
 fn resolve_pat<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, pat: &'tcx hir::Pat) {
@@ -893,10 +905,6 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr:
                 terminating(body.hir_id.local_id);
             }
 
-            hir::ExprKind::Match(..) => {
-                visitor.cx.var_parent = visitor.cx.parent;
-            }
-
             hir::ExprKind::DropTemps(ref expr) => {
                 // `DropTemps(expr)` does not denote a conditional scope.
                 // Rather, we want to achieve the same behavior as `{ let _t = expr; _t }`.

src/librustc_mir/build/block.rs

@@ -23,8 +23,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             safety_mode
         } =
             self.hir.mirror(ast_block);
-        self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), block, move |this| {
-            this.in_scope((region_scope, source_info), LintLevel::Inherited, block, move |this| {
+        self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), move |this| {
+            this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
                 if targeted_by_break {
                     // This is a `break`-able block
                     let exit_block = this.cfg.start_new_block();
@@ -83,9 +83,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 StmtKind::Expr { scope, expr } => {
                     this.block_context.push(BlockFrame::Statement { ignores_expr_result: true });
                     unpack!(block = this.in_opt_scope(
-                        opt_destruction_scope.map(|de|(de, source_info)), block, |this| {
+                        opt_destruction_scope.map(|de|(de, source_info)), |this| {
                             let si = (scope, source_info);
-                            this.in_scope(si, LintLevel::Inherited, block, |this| {
+                            this.in_scope(si, LintLevel::Inherited, |this| {
                                 let expr = this.hir.mirror(expr);
                                 this.stmt_expr(block, expr, Some(stmt_span))
                             })
@@ -113,31 +113,39 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     let remainder_span = remainder_scope.span(this.hir.tcx(),
                                                               &this.hir.region_scope_tree);
 
-                    let scope;
+                    let visibility_scope =
+                        Some(this.new_source_scope(remainder_span, LintLevel::Inherited, None));
 
                     // Evaluate the initializer, if present.
                     if let Some(init) = initializer {
                         let initializer_span = init.span();
 
-                        scope = this.declare_bindings(
-                            None,
-                            remainder_span,
-                            lint_level,
-                            &pattern,
-                            ArmHasGuard(false),
-                            Some((None, initializer_span)),
-                        );
                         unpack!(block = this.in_opt_scope(
-                            opt_destruction_scope.map(|de|(de, source_info)), block, |this| {
+                            opt_destruction_scope.map(|de|(de, source_info)), |this| {
                                 let scope = (init_scope, source_info);
-                                this.in_scope(scope, lint_level, block, |this| {
+                                this.in_scope(scope, lint_level, |this| {
+                                    this.declare_bindings(
+                                        visibility_scope,
+                                        remainder_span,
+                                        &pattern,
+                                        ArmHasGuard(false),
+                                        Some((None, initializer_span)),
+                                    );
                                     this.expr_into_pattern(block, pattern, init)
                                 })
                             }));
                     } else {
-                        scope = this.declare_bindings(
-                            None, remainder_span, lint_level, &pattern,
-                            ArmHasGuard(false), None);
+                        let scope = (init_scope, source_info);
+                        unpack!(this.in_scope(scope, lint_level, |this| {
+                            this.declare_bindings(
+                                visibility_scope,
+                                remainder_span,
+                                &pattern,
+                                ArmHasGuard(false),
+                                None,
+                            );
+                            block.unit()
+                        }));
 
                         debug!("ast_block_stmts: pattern={:?}", pattern);
                         this.visit_bindings(
@@ -149,8 +157,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                             })
                     }
 
-                    // Enter the source scope, after evaluating the initializer.
-                    if let Some(source_scope) = scope {
+                    // Enter the visibility scope, after evaluating the initializer.
+                    if let Some(source_scope) = visibility_scope {
                         this.source_scope = source_scope;
                     }
                 }