Make hir::Stmt a separate struct.

Benefits: - It lets us move the `NodeId` field out of every `hir::StmtKind` variant `NodeId` to a more sensible spot. - It eliminates sadness in `Stmt::fmt`. - It makes `hir::Stmt` match `ast::Stmt`.
rust-lang · Jan 16, 2019 · b2ce5a9 · b2ce5a9
1 parent e2f221c
commit b2ce5a9
Show file tree

Hide file tree

Showing 16 changed files with 92 additions and 87 deletions.
diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs
@@ -99,15 +99,15 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
     }
 
     fn stmt(&mut self, stmt: &hir::Stmt, pred: CFGIndex) -> CFGIndex {
-        let hir_id = self.tcx.hir().node_to_hir_id(stmt.node.id());
+        let hir_id = self.tcx.hir().node_to_hir_id(stmt.id);
         match stmt.node {
-            hir::StmtKind::Decl(ref decl, _) => {
+            hir::StmtKind::Decl(ref decl) => {
                 let exit = self.decl(&decl, pred);
                 self.add_ast_node(hir_id.local_id, &[exit])
             }
 
-            hir::StmtKind::Expr(ref expr, _) |
-            hir::StmtKind::Semi(ref expr, _) => {
+            hir::StmtKind::Expr(ref expr) |
+            hir::StmtKind::Semi(ref expr) => {
                 let exit = self.expr(&expr, pred);
                 self.add_ast_node(hir_id.local_id, &[exit])
             }

diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs
@@ -298,7 +298,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
 
     fn check_stmt_attributes(&self, stmt: &hir::Stmt) {
         // When checking statements ignore expressions, they will be checked later
-        if let hir::StmtKind::Decl(_, _) = stmt.node {
+        if let hir::StmtKind::Decl(..) = stmt.node {
             for attr in stmt.node.attrs() {
                 if attr.check_name("inline") {
                     self.check_inline(attr, &stmt.span, Target::Statement);

diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs
@@ -953,14 +953,13 @@ pub fn walk_block<'v, V: Visitor<'v>>(visitor: &mut V, block: &'v Block) {
 }
 
 pub fn walk_stmt<'v, V: Visitor<'v>>(visitor: &mut V, statement: &'v Stmt) {
+    visitor.visit_id(statement.id);
     match statement.node {
-        StmtKind::Decl(ref declaration, id) => {
-            visitor.visit_id(id);
+        StmtKind::Decl(ref declaration) => {
             visitor.visit_decl(declaration)
         }
-        StmtKind::Expr(ref expression, id) |
-        StmtKind::Semi(ref expression, id) => {
-            visitor.visit_id(id);
+        StmtKind::Expr(ref expression) |
+        StmtKind::Semi(ref expression) => {
             visitor.visit_expr(expression)
         }
     }

diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -4331,10 +4331,11 @@ impl<'a> LoweringContext<'a> {
                         ThinVec::new(),
                     ))
                 };
-                let match_stmt = respan(
-                    head_sp,
-                    hir::StmtKind::Expr(match_expr, self.next_id().node_id)
-                );
+                let match_stmt = hir::Stmt {
+                    id: self.next_id().node_id,
+                    node: hir::StmtKind::Expr(match_expr),
+                    span: head_sp,
+                };
 
                 let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat.id));
 
@@ -4357,10 +4358,11 @@ impl<'a> LoweringContext<'a> {
 
                 let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false));
                 let body_expr = P(self.expr_block(body_block, ThinVec::new()));
-                let body_stmt = respan(
-                    body.span,
-                    hir::StmtKind::Expr(body_expr, self.next_id().node_id)
-                );
+                let body_stmt = hir::Stmt {
+                    id: self.next_id().node_id,
+                    node: hir::StmtKind::Expr(body_expr),
+                    span: body.span,
+                };
 
                 let loop_block = P(self.block_all(
                     e.span,
@@ -4533,24 +4535,24 @@ impl<'a> LoweringContext<'a> {
                 let (l, item_ids) = self.lower_local(l);
                 let mut ids: SmallVec<[hir::Stmt; 1]> = item_ids
                     .into_iter()
-                    .map(|item_id| Spanned {
+                    .map(|item_id| hir::Stmt {
+                        id: self.next_id().node_id,
                         node: hir::StmtKind::Decl(
                             P(Spanned {
                                 node: hir::DeclKind::Item(item_id),
                                 span: s.span,
                             }),
-                            self.next_id().node_id,
                         ),
                         span: s.span,
                     })
                     .collect();
-                ids.push(Spanned {
+                ids.push(hir::Stmt {
+                    id: self.lower_node_id(s.id).node_id,
                     node: hir::StmtKind::Decl(
                         P(Spanned {
                             node: hir::DeclKind::Local(l),
                             span: s.span,
                         }),
-                        self.lower_node_id(s.id).node_id,
                     ),
                     span: s.span,
                 });
@@ -4561,26 +4563,28 @@ impl<'a> LoweringContext<'a> {
                 let mut id = Some(s.id);
                 return self.lower_item_id(it)
                     .into_iter()
-                    .map(|item_id| Spanned {
+                    .map(|item_id| hir::Stmt {
+                        id: id.take()
+                              .map(|id| self.lower_node_id(id).node_id)
+                              .unwrap_or_else(|| self.next_id().node_id),
                         node: hir::StmtKind::Decl(
                             P(Spanned {
                                 node: hir::DeclKind::Item(item_id),
                                 span: s.span,
                             }),
-                            id.take()
-                              .map(|id| self.lower_node_id(id).node_id)
-                              .unwrap_or_else(|| self.next_id().node_id),
                         ),
                         span: s.span,
                     })
                     .collect();
             }
-            StmtKind::Expr(ref e) => Spanned {
-                node: hir::StmtKind::Expr(P(self.lower_expr(e)), self.lower_node_id(s.id).node_id),
+            StmtKind::Expr(ref e) => hir::Stmt {
+                id: self.lower_node_id(s.id).node_id,
+                node: hir::StmtKind::Expr(P(self.lower_expr(e))),
                 span: s.span,
             },
-            StmtKind::Semi(ref e) => Spanned {
-                node: hir::StmtKind::Semi(P(self.lower_expr(e)), self.lower_node_id(s.id).node_id),
+            StmtKind::Semi(ref e) => hir::Stmt {
+                id: self.lower_node_id(s.id).node_id,
+                node: hir::StmtKind::Semi(P(self.lower_expr(e))),
                 span: s.span,
             },
             StmtKind::Mac(..) => panic!("Shouldn't exist here"),
@@ -4806,7 +4810,11 @@ impl<'a> LoweringContext<'a> {
             source,
         });
         let decl = respan(sp, hir::DeclKind::Local(local));
-        respan(sp, hir::StmtKind::Decl(P(decl), self.next_id().node_id))
+        hir::Stmt {
+            id: self.next_id().node_id,
+            node: hir::StmtKind::Decl(P(decl)),
+            span: sp
+        }
     }
 
     fn stmt_let(

diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs
@@ -426,7 +426,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     }
 
     fn visit_stmt(&mut self, stmt: &'hir Stmt) {
-        let id = stmt.node.id();
+        let id = stmt.id;
         self.insert(stmt.span, id, Node::Stmt(stmt));
 
         self.with_parent(id, |this| {

diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
@@ -16,7 +16,7 @@ use util::nodemap::{NodeMap, FxHashSet};
 use mir::mono::Linkage;
 
 use syntax_pos::{Span, DUMMY_SP, symbol::InternedString};
-use syntax::source_map::{self, Spanned};
+use syntax::source_map::Spanned;
 use rustc_target::spec::abi::Abi;
 use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect};
 use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy};
@@ -1144,45 +1144,38 @@ impl UnOp {
 }
 
 /// A statement
-pub type Stmt = Spanned<StmtKind>;
+#[derive(Clone, RustcEncodable, RustcDecodable)]
+pub struct Stmt {
+    pub id: NodeId,
+    pub node: StmtKind,
+    pub span: Span,
+}
 
-impl fmt::Debug for StmtKind {
+impl fmt::Debug for Stmt {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        // Sadness.
-        let spanned = source_map::dummy_spanned(self.clone());
-        write!(f,
-               "stmt({}: {})",
-               spanned.node.id(),
-               print::to_string(print::NO_ANN, |s| s.print_stmt(&spanned)))
+        write!(f, "stmt({}: {})", self.id,
+               print::to_string(print::NO_ANN, |s| s.print_stmt(self)))
     }
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable)]
 pub enum StmtKind {
     /// Could be an item or a local (let) binding:
-    Decl(P<Decl>, NodeId),
+    Decl(P<Decl>),
 
     /// Expr without trailing semi-colon (must have unit type):
-    Expr(P<Expr>, NodeId),
+    Expr(P<Expr>),
 
     /// Expr with trailing semi-colon (may have any type):
-    Semi(P<Expr>, NodeId),
+    Semi(P<Expr>),
 }
 
 impl StmtKind {
     pub fn attrs(&self) -> &[Attribute] {
         match *self {
-            StmtKind::Decl(ref d, _) => d.node.attrs(),
-            StmtKind::Expr(ref e, _) |
-            StmtKind::Semi(ref e, _) => &e.attrs,
-        }
-    }
-
-    pub fn id(&self) -> NodeId {
-        match *self {
-            StmtKind::Decl(_, id) |
-            StmtKind::Expr(_, id) |
-            StmtKind::Semi(_, id) => id,
+            StmtKind::Decl(ref d) => d.node.attrs(),
+            StmtKind::Expr(ref e) |
+            StmtKind::Semi(ref e) => &e.attrs,
         }
     }
 }

diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs
@@ -992,14 +992,14 @@ impl<'a> State<'a> {
     pub fn print_stmt(&mut self, st: &hir::Stmt) -> io::Result<()> {
         self.maybe_print_comment(st.span.lo())?;
         match st.node {
-            hir::StmtKind::Decl(ref decl, _) => {
+            hir::StmtKind::Decl(ref decl) => {
                 self.print_decl(&decl)?;
             }
-            hir::StmtKind::Expr(ref expr, _) => {
+            hir::StmtKind::Expr(ref expr) => {
                 self.space_if_not_bol()?;
                 self.print_expr(&expr)?;
             }
-            hir::StmtKind::Semi(ref expr, _) => {
+            hir::StmtKind::Semi(ref expr) => {
                 self.space_if_not_bol()?;
                 self.print_expr(&expr)?;
                 self.s.word(";")?;
@@ -2401,13 +2401,13 @@ fn expr_requires_semi_to_be_stmt(e: &hir::Expr) -> bool {
 /// seen the semicolon, and thus don't need another.
 fn stmt_ends_with_semi(stmt: &hir::StmtKind) -> bool {
     match *stmt {
-        hir::StmtKind::Decl(ref d, _) => {
+        hir::StmtKind::Decl(ref d) => {
             match d.node {
                 hir::DeclKind::Local(_) => true,
                 hir::DeclKind::Item(_) => false,
             }
         }
-        hir::StmtKind::Expr(ref e, _) => {
+        hir::StmtKind::Expr(ref e) => {
             expr_requires_semi_to_be_stmt(&e)
         }
         hir::StmtKind::Semi(..) => {

diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs
@@ -483,7 +483,12 @@ impl_stable_hash_for!(enum hir::UnOp {
     UnNeg
 });
 
-impl_stable_hash_for_spanned!(hir::StmtKind);
+impl_stable_hash_for!(struct hir::Stmt {
+    id,
+    node,
+    span,
+});
+
 
 impl_stable_hash_for!(struct hir::Local {
     pat,
@@ -941,9 +946,9 @@ impl_stable_hash_for!(enum hir::ForeignItemKind {
 });
 
 impl_stable_hash_for!(enum hir::StmtKind {
-    Decl(decl, id),
-    Expr(expr, id),
-    Semi(expr, id)
+    Decl(decl),
+    Expr(expr),
+    Semi(expr)
 });
 
 impl_stable_hash_for!(struct hir::Arg {

diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs
@@ -589,7 +589,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
 
     fn walk_stmt(&mut self, stmt: &hir::Stmt) {
         match stmt.node {
-            hir::StmtKind::Decl(ref decl, _) => {
+            hir::StmtKind::Decl(ref decl) => {
                 match decl.node {
                     hir::DeclKind::Local(ref local) => {
                         self.walk_local(&local);
@@ -602,8 +602,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
                 }
             }
 
-            hir::StmtKind::Expr(ref expr, _) |
-            hir::StmtKind::Semi(ref expr, _) => {
+            hir::StmtKind::Expr(ref expr) |
+            hir::StmtKind::Semi(ref expr) => {
                 self.consume_expr(&expr);
             }
         }

diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs
@@ -956,11 +956,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
     fn propagate_through_stmt(&mut self, stmt: &hir::Stmt, succ: LiveNode)
                               -> LiveNode {
         match stmt.node {
-            hir::StmtKind::Decl(ref decl, _) => {
+            hir::StmtKind::Decl(ref decl) => {
                 self.propagate_through_decl(&decl, succ)
             }
 
-            hir::StmtKind::Expr(ref expr, _) | hir::StmtKind::Semi(ref expr, _) => {
+            hir::StmtKind::Expr(ref expr) | hir::StmtKind::Semi(ref expr) => {
                 self.propagate_through_expr(&expr, succ)
             }
         }

diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs
@@ -835,7 +835,7 @@ fn resolve_pat<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, pat: &
 }
 
 fn resolve_stmt<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, stmt: &'tcx hir::Stmt) {
-    let stmt_id = visitor.tcx.hir().node_to_hir_id(stmt.node.id()).local_id;
+    let stmt_id = visitor.tcx.hir().node_to_hir_id(stmt.id).local_id;
     debug!("resolve_stmt(stmt.id={:?})", stmt_id);
 
     // Every statement will clean up the temporaries created during

diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs
@@ -41,7 +41,7 @@ impl LintPass for UnusedResults {
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
     fn check_stmt(&mut self, cx: &LateContext, s: &hir::Stmt) {
         let expr = match s.node {
-            hir::StmtKind::Semi(ref expr, _) => &**expr,
+            hir::StmtKind::Semi(ref expr) => &**expr,
             _ => return,
         };
 
@@ -205,7 +205,7 @@ impl LintPass for PathStatements {
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PathStatements {
     fn check_stmt(&mut self, cx: &LateContext, s: &hir::Stmt) {
-        if let hir::StmtKind::Semi(ref expr, _) = s.node {
+        if let hir::StmtKind::Semi(ref expr) = s.node {
             if let hir::ExprKind::Path(_) = expr.node {
                 cx.span_lint(PATH_STATEMENTS, s.span, "path statement with no effect");
             }

diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs
@@ -46,12 +46,12 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
                                 -> Vec<StmtRef<'tcx>> {
     let mut result = vec![];
     for (index, stmt) in stmts.iter().enumerate() {
-        let hir_id = cx.tcx.hir().node_to_hir_id(stmt.node.id());
+        let hir_id = cx.tcx.hir().node_to_hir_id(stmt.id);
         let opt_dxn_ext = cx.region_scope_tree.opt_destruction_scope(hir_id.local_id);
-        let stmt_span = StatementSpan(cx.tcx.hir().span(stmt.node.id()));
+        let stmt_span = StatementSpan(cx.tcx.hir().span(stmt.id));
         match stmt.node {
-            hir::StmtKind::Expr(ref expr, _) |
-            hir::StmtKind::Semi(ref expr, _) => {
+            hir::StmtKind::Expr(ref expr) |
+            hir::StmtKind::Semi(ref expr) => {
                 result.push(StmtRef::Mirror(Box::new(Stmt {
                     kind: StmtKind::Expr {
                         scope: region::Scope {
@@ -64,7 +64,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
                     span: stmt_span,
                 })))
             }
-            hir::StmtKind::Decl(ref decl, _) => {
+            hir::StmtKind::Decl(ref decl) => {
                 match decl.node {
                     hir::DeclKind::Item(..) => {
                         // ignore for purposes of the MIR

diff --git a/src/librustc_passes/hir_stats.rs b/src/librustc_passes/hir_stats.rs
@@ -144,7 +144,7 @@ impl<'v> hir_visit::Visitor<'v> for StatCollector<'v> {
     }
 
     fn visit_stmt(&mut self, s: &'v hir::Stmt) {
-        self.record("Stmt", Id::Node(s.node.id()), s);
+        self.record("Stmt", Id::Node(s.id), s);
         hir_visit::walk_stmt(self, s)
     }