syntax/hir: give loop labels a span

This makes the "shadowing labels" warning *not* print the entire loop
as a span, but only the lifetime.

Also makes #31719 go away, but does not fix its root cause (the span
of the expanded loop is still wonky, but not used anymore).

Author: birkenfeld

src/librustc/hir/fold.rs

@@ -1009,11 +1009,15 @@ pub fn noop_fold_expr<T: Folder>(Expr { id, node, span, attrs }: Expr, folder: &
             ExprWhile(cond, body, opt_name) => {
                 ExprWhile(folder.fold_expr(cond),
                           folder.fold_block(body),
-                          opt_name.map(|i| folder.fold_name(i)))
+                          opt_name.map(|label| {
+                              respan(folder.new_span(label.span), folder.fold_name(label.node))
+                          }))
             }
             ExprLoop(body, opt_name) => {
                 ExprLoop(folder.fold_block(body),
-                         opt_name.map(|i| folder.fold_name(i)))
+                         opt_name.map(|label| {
+                             respan(folder.new_span(label.span), folder.fold_name(label.node))
+                         }))
             }
             ExprMatch(expr, arms, source) => {
                 ExprMatch(folder.fold_expr(expr),

src/librustc/hir/intravisit.rs

@@ -28,7 +28,7 @@
 use syntax::abi::Abi;
 use syntax::ast::{NodeId, CRATE_NODE_ID, Name, Attribute};
 use syntax::attr::ThinAttributesExt;
-use syntax::codemap::Span;
+use syntax::codemap::{Span, Spanned};
 use hir::*;
 
 use std::cmp;
@@ -203,11 +203,17 @@ pub trait Visitor<'v> : Sized {
 }
 
 pub fn walk_opt_name<'v, V: Visitor<'v>>(visitor: &mut V, span: Span, opt_name: Option<Name>) {
-    for name in opt_name {
+    if let Some(name) = opt_name {
         visitor.visit_name(span, name);
     }
 }
 
+pub fn walk_opt_sp_name<'v, V: Visitor<'v>>(visitor: &mut V, opt_sp_name: &Option<Spanned<Name>>) {
+    if let Some(ref sp_name) = *opt_sp_name {
+        visitor.visit_name(sp_name.span, sp_name.node);
+    }
+}
+
 /// Walks the contents of a crate. See also `Crate::visit_all_items`.
 pub fn walk_crate<'v, V: Visitor<'v>>(visitor: &mut V, krate: &'v Crate) {
     visitor.visit_mod(&krate.module, krate.span, CRATE_NODE_ID);
@@ -737,14 +743,14 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
             visitor.visit_block(if_block);
             walk_list!(visitor, visit_expr, optional_else);
         }
-        ExprWhile(ref subexpression, ref block, opt_name) => {
+        ExprWhile(ref subexpression, ref block, ref opt_sp_name) => {
             visitor.visit_expr(subexpression);
             visitor.visit_block(block);
-            walk_opt_name(visitor, expression.span, opt_name)
+            walk_opt_sp_name(visitor, opt_sp_name);
         }
-        ExprLoop(ref block, opt_name) => {
+        ExprLoop(ref block, ref opt_sp_name) => {
             visitor.visit_block(block);
-            walk_opt_name(visitor, expression.span, opt_name)
+            walk_opt_sp_name(visitor, opt_sp_name);
         }
         ExprMatch(ref subexpression, ref arms, _) => {
             visitor.visit_expr(subexpression);
@@ -784,9 +790,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
             visitor.visit_path(path, expression.id)
         }
         ExprBreak(ref opt_sp_name) | ExprAgain(ref opt_sp_name) => {
-            for sp_name in opt_sp_name {
-                visitor.visit_name(sp_name.span, sp_name.node);
-            }
+            walk_opt_sp_name(visitor, opt_sp_name);
         }
         ExprRet(ref optional_expression) => {
             walk_list!(visitor, visit_expr, optional_expression);

src/librustc/hir/lowering.rs

@@ -192,6 +192,10 @@ impl<'a> LoweringContext<'a> {
         }
     }
 
+    fn lower_opt_sp_ident(&mut self, o_id: Option<Spanned<Ident>>) -> Option<Spanned<Name>> {
+        o_id.map(|sp_ident| respan(sp_ident.span, self.lower_ident(sp_ident.node)))
+    }
+
     fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
         attrs.clone().into()
     }
@@ -1122,11 +1126,10 @@ impl<'a> LoweringContext<'a> {
                 }
                 ExprKind::While(ref cond, ref body, opt_ident) => {
                     hir::ExprWhile(self.lower_expr(cond), self.lower_block(body),
-                                   opt_ident.map(|ident| self.lower_ident(ident)))
+                                   self.lower_opt_sp_ident(opt_ident))
                 }
                 ExprKind::Loop(ref body, opt_ident) => {
-                    hir::ExprLoop(self.lower_block(body),
-                                  opt_ident.map(|ident| self.lower_ident(ident)))
+                    hir::ExprLoop(self.lower_block(body), self.lower_opt_sp_ident(opt_ident))
                 }
                 ExprKind::Match(ref expr, ref arms) => {
                     hir::ExprMatch(self.lower_expr(expr),
@@ -1243,12 +1246,8 @@ impl<'a> LoweringContext<'a> {
                     };
                     hir::ExprPath(hir_qself, self.lower_path_full(path, rename))
                 }
-                ExprKind::Break(opt_ident) => hir::ExprBreak(opt_ident.map(|sp_ident| {
-                    respan(sp_ident.span, self.lower_ident(sp_ident.node))
-                })),
-                ExprKind::Again(opt_ident) => hir::ExprAgain(opt_ident.map(|sp_ident| {
-                    respan(sp_ident.span, self.lower_ident(sp_ident.node))
-                })),
+                ExprKind::Break(opt_ident) => hir::ExprBreak(self.lower_opt_sp_ident(opt_ident)),
+                ExprKind::Again(opt_ident) => hir::ExprAgain(self.lower_opt_sp_ident(opt_ident)),
                 ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| self.lower_expr(x))),
                 ExprKind::InlineAsm(InlineAsm {
                         ref inputs,
@@ -1422,8 +1421,7 @@ impl<'a> LoweringContext<'a> {
 
                     // `[opt_ident]: loop { ... }`
                     let loop_block = self.block_expr(match_expr);
-                    let loop_expr = hir::ExprLoop(loop_block,
-                                                  opt_ident.map(|ident| self.lower_ident(ident)));
+                    let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident));
                     // add attributes to the outer returned expr node
                     let attrs = e.attrs.clone();
                     return P(hir::Expr { id: e.id, node: loop_expr, span: e.span, attrs: attrs });
@@ -1503,8 +1501,7 @@ impl<'a> LoweringContext<'a> {
 
                     // `[opt_ident]: loop { ... }`
                     let loop_block = self.block_expr(match_expr);
-                    let loop_expr = hir::ExprLoop(loop_block,
-                                                  opt_ident.map(|ident| self.lower_ident(ident)));
+                    let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident));
                     let loop_expr =
                         P(hir::Expr { id: e.id, node: loop_expr, span: e.span, attrs: None });
 

src/librustc/hir/mod.rs

@@ -873,11 +873,11 @@ pub enum Expr_ {
     /// A while loop, with an optional label
     ///
     /// `'label: while expr { block }`
-    ExprWhile(P<Expr>, P<Block>, Option<Name>),
+    ExprWhile(P<Expr>, P<Block>, Option<Spanned<Name>>),
     /// Conditionless loop (can be exited with break, continue, or return)
     ///
     /// `'label: loop { block }`
-    ExprLoop(P<Block>, Option<Name>),
+    ExprLoop(P<Block>, Option<Spanned<Name>>),
     /// A `match` block, with a source that indicates whether or not it is
     /// the result of a desugaring, and if so, which kind.
     ExprMatch(P<Expr>, HirVec<Arm>, MatchSource),

src/librustc/hir/print.rs

@@ -1351,19 +1351,19 @@ impl<'a> State<'a> {
             hir::ExprIf(ref test, ref blk, ref elseopt) => {
                 self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e))?;
             }
-            hir::ExprWhile(ref test, ref blk, opt_name) => {
-                if let Some(name) = opt_name {
-                    self.print_name(name)?;
+            hir::ExprWhile(ref test, ref blk, opt_sp_name) => {
+                if let Some(sp_name) = opt_sp_name {
+                    self.print_name(sp_name.node)?;
                     self.word_space(":")?;
                 }
                 self.head("while")?;
                 self.print_expr(&test)?;
                 space(&mut self.s)?;
                 self.print_block(&blk)?;
             }
-            hir::ExprLoop(ref blk, opt_name) => {
-                if let Some(name) = opt_name {
-                    self.print_name(name)?;
+            hir::ExprLoop(ref blk, opt_sp_name) => {
+                if let Some(sp_name) = opt_sp_name {
+                    self.print_name(sp_name.node)?;
                     self.word_space(":")?;
                 }
                 self.head("loop")?;

src/librustc/middle/resolve_lifetime.rs

@@ -404,23 +404,23 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) {
             if let hir::ExprClosure(..) = ex.node {
                 return
             }
-            if let Some(label) = expression_label(ex) {
+            if let Some((label, label_span)) = expression_label(ex) {
                 for &(prior, prior_span) in &self.labels_in_fn[..] {
                     // FIXME (#24278): non-hygienic comparison
                     if label == prior {
                         signal_shadowing_problem(self.sess,
                                                  label,
                                                  original_label(prior_span),
-                                                 shadower_label(ex.span));
+                                                 shadower_label(label_span));
                     }
                 }
 
                 check_if_label_shadows_lifetime(self.sess,
                                                 self.scope,
                                                 label,
-                                                ex.span);
+                                                label_span);
 
-                self.labels_in_fn.push((label, ex.span));
+                self.labels_in_fn.push((label, label_span));
             }
             intravisit::walk_expr(self, ex)
         }
@@ -430,10 +430,11 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) {
         }
     }
 
-    fn expression_label(ex: &hir::Expr) -> Option<ast::Name> {
+    fn expression_label(ex: &hir::Expr) -> Option<(ast::Name, Span)> {
         match ex.node {
             hir::ExprWhile(_, _, Some(label)) |
-            hir::ExprLoop(_, Some(label)) => Some(label.unhygienize()),
+            hir::ExprLoop(_, Some(label)) => Some((label.node.unhygienize(),
+                                                   label.span)),
             _ => None,
         }
     }

src/librustc_incremental/calculate_svh.rs

@@ -251,7 +251,7 @@ mod svh_visitor {
             ExprType(..)             => SawExprType,
             ExprIf(..)               => SawExprIf,
             ExprWhile(..)            => SawExprWhile,
-            ExprLoop(_, id)          => SawExprLoop(id.map(|id| id.as_str())),
+            ExprLoop(_, id)          => SawExprLoop(id.map(|id| id.node.as_str())),
             ExprMatch(..)            => SawExprMatch,
             ExprClosure(..)          => SawExprClosure,
             ExprBlock(..)            => SawExprBlock,

src/librustc_resolve/lib.rs

@@ -3086,7 +3086,7 @@ impl<'a> Resolver<'a> {
 
                     {
                         let rib = this.label_ribs.last_mut().unwrap();
-                        rib.bindings.insert(mtwt::resolve(label), def);
+                        rib.bindings.insert(mtwt::resolve(label.node), def);
                     }
 
                     visit::walk_expr(this, expr);
@@ -3131,7 +3131,7 @@ impl<'a> Resolver<'a> {
                 self.value_ribs.push(Rib::new(NormalRibKind));
                 self.resolve_pattern(pattern, RefutableMode, &mut HashMap::new());
 
-                self.resolve_labeled_block(label, expr.id, block);
+                self.resolve_labeled_block(label.map(|l| l.node), expr.id, block);
 
                 self.value_ribs.pop();
             }
@@ -3141,7 +3141,7 @@ impl<'a> Resolver<'a> {
                 self.value_ribs.push(Rib::new(NormalRibKind));
                 self.resolve_pattern(pattern, LocalIrrefutableMode, &mut HashMap::new());
 
-                self.resolve_labeled_block(label, expr.id, block);
+                self.resolve_labeled_block(label.map(|l| l.node), expr.id, block);
 
                 self.value_ribs.pop();
             }

src/libsyntax/ast.rs

@@ -1007,23 +1007,23 @@ pub enum ExprKind {
     /// A while loop, with an optional label
     ///
     /// `'label: while expr { block }`
-    While(P<Expr>, P<Block>, Option<Ident>),
+    While(P<Expr>, P<Block>, Option<SpannedIdent>),
     /// A while-let loop, with an optional label
     ///
     /// `'label: while let pat = expr { block }`
     ///
     /// This is desugared to a combination of `loop` and `match` expressions.
-    WhileLet(P<Pat>, P<Expr>, P<Block>, Option<Ident>),
+    WhileLet(P<Pat>, P<Expr>, P<Block>, Option<SpannedIdent>),
     /// A for loop, with an optional label
     ///
     /// `'label: for pat in expr { block }`
     ///
     /// This is desugared to a combination of `loop` and `match` expressions.
-    ForLoop(P<Pat>, P<Expr>, P<Block>, Option<Ident>),
+    ForLoop(P<Pat>, P<Expr>, P<Block>, Option<SpannedIdent>),
     /// Conditionless loop (can be exited with break, continue, or return)
     ///
     /// `'label: loop { block }`
-    Loop(P<Block>, Option<Ident>),
+    Loop(P<Block>, Option<SpannedIdent>),
     /// A `match` block.
     Match(P<Expr>, Vec<Arm>),
     /// A closure (for example, `move |a, b, c| {a + b + c}`)

src/libsyntax/ext/expand.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use ast::{Block, Crate, DeclKind, PatKind};
-use ast::{Local, Ident, Mac_, Name};
+use ast::{Local, Ident, Mac_, Name, SpannedIdent};
 use ast::{MacStmtStyle, Mrk, Stmt, StmtKind, ItemKind};
 use ast::TokenTree;
 use ast;
@@ -277,20 +277,20 @@ fn expand_mac_invoc<T, F, G>(mac: ast::Mac,
 /// body is in a block enclosed by loop head so the renaming of loop label
 /// must be propagated to the enclosed context.
 fn expand_loop_block(loop_block: P<Block>,
-                     opt_ident: Option<Ident>,
-                     fld: &mut MacroExpander) -> (P<Block>, Option<Ident>) {
+                     opt_ident: Option<SpannedIdent>,
+                     fld: &mut MacroExpander) -> (P<Block>, Option<SpannedIdent>) {
     match opt_ident {
         Some(label) => {
-            let new_label = fresh_name(label);
-            let rename = (label, new_label);
+            let new_label = fresh_name(label.node);
+            let rename = (label.node, new_label);
 
             // The rename *must not* be added to the pending list of current
             // syntax context otherwise an unrelated `break` or `continue` in
             // the same context will pick that up in the deferred renaming pass
             // and be renamed incorrectly.
             let mut rename_list = vec!(rename);
             let mut rename_fld = IdentRenamer{renames: &mut rename_list};
-            let renamed_ident = rename_fld.fold_ident(label);
+            let renamed_ident = rename_fld.fold_ident(label.node);
 
             // The rename *must* be added to the enclosed syntax context for
             // `break` or `continue` to pick up because by definition they are
@@ -300,7 +300,7 @@ fn expand_loop_block(loop_block: P<Block>,
             let expanded_block = expand_block_elts(loop_block, fld);
             fld.cx.syntax_env.pop_frame();
 
-            (expanded_block, Some(renamed_ident))
+            (expanded_block, Some(Spanned { node: renamed_ident, span: label.span }))
         }
         None => (fld.fold_block(loop_block), opt_ident)
     }