Skip to content

Commit a70880f

Browse files
ManishearthManishearth
committed
Rollup merge of #33351 - birkenfeld:loop-label-spans, r=pnkfelix
 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).
2 parents 97e3a24 + 2e812e1 commit a70880f

File tree

14 files changed

+99
-83
lines changed

14 files changed

+99
-83
lines changed

‎src/librustc/hir/fold.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,11 +1009,15 @@ pub fn noop_fold_expr<T: Folder>(Expr { id, node, span, attrs }: Expr, folder: &
10091009
ExprWhile(cond, body, opt_name) => {
10101010
ExprWhile(folder.fold_expr(cond),
10111011
folder.fold_block(body),
1012-
opt_name.map(|i| folder.fold_name(i)))
1012+
opt_name.map(|label| {
1013+
respan(folder.new_span(label.span), folder.fold_name(label.node))
1014+
}))
10131015
}
10141016
ExprLoop(body, opt_name) => {
10151017
ExprLoop(folder.fold_block(body),
1016-
opt_name.map(|i| folder.fold_name(i)))
1018+
opt_name.map(|label| {
1019+
respan(folder.new_span(label.span), folder.fold_name(label.node))
1020+
}))
10171021
}
10181022
ExprMatch(expr, arms, source) => {
10191023
ExprMatch(folder.fold_expr(expr),

‎src/librustc/hir/intravisit.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
use syntax::abi::Abi;
2929
use syntax::ast::{NodeId, CRATE_NODE_ID, Name, Attribute};
3030
use syntax::attr::ThinAttributesExt;
31-
use syntax::codemap::Span;
31+
use syntax::codemap::{Span, Spanned};
3232
use hir::*;
3333

3434
use std::cmp;
@@ -203,11 +203,17 @@ pub trait Visitor<'v> : Sized {
203203
}
204204

205205
pub fn walk_opt_name<'v, V: Visitor<'v>>(visitor: &mut V, span: Span, opt_name: Option<Name>) {
206-
for name in opt_name {
206+
if let Some(name) = opt_name {
207207
visitor.visit_name(span, name);
208208
}
209209
}
210210

211+
pub fn walk_opt_sp_name<'v, V: Visitor<'v>>(visitor: &mut V, opt_sp_name: &Option<Spanned<Name>>) {
212+
if let Some(ref sp_name) = *opt_sp_name {
213+
visitor.visit_name(sp_name.span, sp_name.node);
214+
}
215+
}
216+
211217
/// Walks the contents of a crate. See also `Crate::visit_all_items`.
212218
pub fn walk_crate<'v, V: Visitor<'v>>(visitor: &mut V, krate: &'v Crate) {
213219
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) {
737743
visitor.visit_block(if_block);
738744
walk_list!(visitor, visit_expr, optional_else);
739745
}
740-
ExprWhile(ref subexpression, ref block, opt_name) => {
746+
ExprWhile(ref subexpression, ref block, ref opt_sp_name) => {
741747
visitor.visit_expr(subexpression);
742748
visitor.visit_block(block);
743-
walk_opt_name(visitor, expression.span, opt_name)
749+
walk_opt_sp_name(visitor, opt_sp_name);
744750
}
745-
ExprLoop(ref block, opt_name) => {
751+
ExprLoop(ref block, ref opt_sp_name) => {
746752
visitor.visit_block(block);
747-
walk_opt_name(visitor, expression.span, opt_name)
753+
walk_opt_sp_name(visitor, opt_sp_name);
748754
}
749755
ExprMatch(ref subexpression, ref arms, _) => {
750756
visitor.visit_expr(subexpression);
@@ -784,9 +790,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
784790
visitor.visit_path(path, expression.id)
785791
}
786792
ExprBreak(ref opt_sp_name) | ExprAgain(ref opt_sp_name) => {
787-
for sp_name in opt_sp_name {
788-
visitor.visit_name(sp_name.span, sp_name.node);
789-
}
793+
walk_opt_sp_name(visitor, opt_sp_name);
790794
}
791795
ExprRet(ref optional_expression) => {
792796
walk_list!(visitor, visit_expr, optional_expression);

‎src/librustc/hir/lowering.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,10 @@ impl<'a> LoweringContext<'a> {
192192
}
193193
}
194194

195+
fn lower_opt_sp_ident(&mut self, o_id: Option<Spanned<Ident>>) -> Option<Spanned<Name>> {
196+
o_id.map(|sp_ident| respan(sp_ident.span, self.lower_ident(sp_ident.node)))
197+
}
198+
195199
fn lower_attrs(&mut self, attrs: &Vec<Attribute>) -> hir::HirVec<Attribute> {
196200
attrs.clone().into()
197201
}
@@ -1122,11 +1126,10 @@ impl<'a> LoweringContext<'a> {
11221126
}
11231127
ExprKind::While(ref cond, ref body, opt_ident) => {
11241128
hir::ExprWhile(self.lower_expr(cond), self.lower_block(body),
1125-
opt_ident.map(|ident| self.lower_ident(ident)))
1129+
self.lower_opt_sp_ident(opt_ident))
11261130
}
11271131
ExprKind::Loop(ref body, opt_ident) => {
1128-
hir::ExprLoop(self.lower_block(body),
1129-
opt_ident.map(|ident| self.lower_ident(ident)))
1132+
hir::ExprLoop(self.lower_block(body), self.lower_opt_sp_ident(opt_ident))
11301133
}
11311134
ExprKind::Match(ref expr, ref arms) => {
11321135
hir::ExprMatch(self.lower_expr(expr),
@@ -1243,12 +1246,8 @@ impl<'a> LoweringContext<'a> {
12431246
};
12441247
hir::ExprPath(hir_qself, self.lower_path_full(path, rename))
12451248
}
1246-
ExprKind::Break(opt_ident) => hir::ExprBreak(opt_ident.map(|sp_ident| {
1247-
respan(sp_ident.span, self.lower_ident(sp_ident.node))
1248-
})),
1249-
ExprKind::Again(opt_ident) => hir::ExprAgain(opt_ident.map(|sp_ident| {
1250-
respan(sp_ident.span, self.lower_ident(sp_ident.node))
1251-
})),
1249+
ExprKind::Break(opt_ident) => hir::ExprBreak(self.lower_opt_sp_ident(opt_ident)),
1250+
ExprKind::Again(opt_ident) => hir::ExprAgain(self.lower_opt_sp_ident(opt_ident)),
12521251
ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| self.lower_expr(x))),
12531252
ExprKind::InlineAsm(InlineAsm {
12541253
ref inputs,
@@ -1422,8 +1421,7 @@ impl<'a> LoweringContext<'a> {
14221421

14231422
// `[opt_ident]: loop { ... }`
14241423
let loop_block = self.block_expr(match_expr);
1425-
let loop_expr = hir::ExprLoop(loop_block,
1426-
opt_ident.map(|ident| self.lower_ident(ident)));
1424+
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident));
14271425
// add attributes to the outer returned expr node
14281426
let attrs = e.attrs.clone();
14291427
return P(hir::Expr { id: e.id, node: loop_expr, span: e.span, attrs: attrs });
@@ -1503,8 +1501,7 @@ impl<'a> LoweringContext<'a> {
15031501

15041502
// `[opt_ident]: loop { ... }`
15051503
let loop_block = self.block_expr(match_expr);
1506-
let loop_expr = hir::ExprLoop(loop_block,
1507-
opt_ident.map(|ident| self.lower_ident(ident)));
1504+
let loop_expr = hir::ExprLoop(loop_block, self.lower_opt_sp_ident(opt_ident));
15081505
let loop_expr =
15091506
P(hir::Expr { id: e.id, node: loop_expr, span: e.span, attrs: None });
15101507

‎src/librustc/hir/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -873,11 +873,11 @@ pub enum Expr_ {
873873
/// A while loop, with an optional label
874874
///
875875
/// `'label: while expr { block }`
876-
ExprWhile(P<Expr>, P<Block>, Option<Name>),
876+
ExprWhile(P<Expr>, P<Block>, Option<Spanned<Name>>),
877877
/// Conditionless loop (can be exited with break, continue, or return)
878878
///
879879
/// `'label: loop { block }`
880-
ExprLoop(P<Block>, Option<Name>),
880+
ExprLoop(P<Block>, Option<Spanned<Name>>),
881881
/// A `match` block, with a source that indicates whether or not it is
882882
/// the result of a desugaring, and if so, which kind.
883883
ExprMatch(P<Expr>, HirVec<Arm>, MatchSource),

‎src/librustc/hir/print.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1351,19 +1351,19 @@ impl<'a> State<'a> {
13511351
hir::ExprIf(ref test, ref blk, ref elseopt) => {
13521352
self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e))?;
13531353
}
1354-
hir::ExprWhile(ref test, ref blk, opt_name) => {
1355-
if let Some(name) = opt_name {
1356-
self.print_name(name)?;
1354+
hir::ExprWhile(ref test, ref blk, opt_sp_name) => {
1355+
if let Some(sp_name) = opt_sp_name {
1356+
self.print_name(sp_name.node)?;
13571357
self.word_space(":")?;
13581358
}
13591359
self.head("while")?;
13601360
self.print_expr(&test)?;
13611361
space(&mut self.s)?;
13621362
self.print_block(&blk)?;
13631363
}
1364-
hir::ExprLoop(ref blk, opt_name) => {
1365-
if let Some(name) = opt_name {
1366-
self.print_name(name)?;
1364+
hir::ExprLoop(ref blk, opt_sp_name) => {
1365+
if let Some(sp_name) = opt_sp_name {
1366+
self.print_name(sp_name.node)?;
13671367
self.word_space(":")?;
13681368
}
13691369
self.head("loop")?;

‎src/librustc/middle/resolve_lifetime.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -404,23 +404,23 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) {
404404
if let hir::ExprClosure(..) = ex.node {
405405
return
406406
}
407-
if let Some(label) = expression_label(ex) {
407+
if let Some((label, label_span)) = expression_label(ex) {
408408
for &(prior, prior_span) in &self.labels_in_fn[..] {
409409
// FIXME (#24278): non-hygienic comparison
410410
if label == prior {
411411
signal_shadowing_problem(self.sess,
412412
label,
413413
original_label(prior_span),
414-
shadower_label(ex.span));
414+
shadower_label(label_span));
415415
}
416416
}
417417

418418
check_if_label_shadows_lifetime(self.sess,
419419
self.scope,
420420
label,
421-
ex.span);
421+
label_span);
422422

423-
self.labels_in_fn.push((label, ex.span));
423+
self.labels_in_fn.push((label, label_span));
424424
}
425425
intravisit::walk_expr(self, ex)
426426
}
@@ -430,10 +430,11 @@ fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v hir::Block) {
430430
}
431431
}
432432

433-
fn expression_label(ex: &hir::Expr) -> Option<ast::Name> {
433+
fn expression_label(ex: &hir::Expr) -> Option<(ast::Name, Span)> {
434434
match ex.node {
435435
hir::ExprWhile(_, _, Some(label)) |
436-
hir::ExprLoop(_, Some(label)) => Some(label.unhygienize()),
436+
hir::ExprLoop(_, Some(label)) => Some((label.node.unhygienize(),
437+
label.span)),
437438
_ => None,
438439
}
439440
}

‎src/librustc_incremental/calculate_svh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ mod svh_visitor {
251251
ExprType(..) => SawExprType,
252252
ExprIf(..) => SawExprIf,
253253
ExprWhile(..) => SawExprWhile,
254-
ExprLoop(_, id) => SawExprLoop(id.map(|id| id.as_str())),
254+
ExprLoop(_, id) => SawExprLoop(id.map(|id| id.node.as_str())),
255255
ExprMatch(..) => SawExprMatch,
256256
ExprClosure(..) => SawExprClosure,
257257
ExprBlock(..) => SawExprBlock,

‎src/librustc_resolve/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3128,7 +3128,7 @@ impl<'a> Resolver<'a> {
31283128

31293129
{
31303130
let rib = this.label_ribs.last_mut().unwrap();
3131-
rib.bindings.insert(mtwt::resolve(label), def);
3131+
rib.bindings.insert(mtwt::resolve(label.node), def);
31323132
}
31333133

31343134
visit::walk_expr(this, expr);
@@ -3173,7 +3173,7 @@ impl<'a> Resolver<'a> {
31733173
self.value_ribs.push(Rib::new(NormalRibKind));
31743174
self.resolve_pattern(pattern, RefutableMode, &mut HashMap::new());
31753175

3176-
self.resolve_labeled_block(label, expr.id, block);
3176+
self.resolve_labeled_block(label.map(|l| l.node), expr.id, block);
31773177

31783178
self.value_ribs.pop();
31793179
}
@@ -3183,7 +3183,7 @@ impl<'a> Resolver<'a> {
31833183
self.value_ribs.push(Rib::new(NormalRibKind));
31843184
self.resolve_pattern(pattern, LocalIrrefutableMode, &mut HashMap::new());
31853185

3186-
self.resolve_labeled_block(label, expr.id, block);
3186+
self.resolve_labeled_block(label.map(|l| l.node), expr.id, block);
31873187

31883188
self.value_ribs.pop();
31893189
}

‎src/libsyntax/ast.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,23 +1007,23 @@ pub enum ExprKind {
10071007
/// A while loop, with an optional label
10081008
///
10091009
/// `'label: while expr { block }`
1010-
While(P<Expr>, P<Block>, Option<Ident>),
1010+
While(P<Expr>, P<Block>, Option<SpannedIdent>),
10111011
/// A while-let loop, with an optional label
10121012
///
10131013
/// `'label: while let pat = expr { block }`
10141014
///
10151015
/// This is desugared to a combination of `loop` and `match` expressions.
1016-
WhileLet(P<Pat>, P<Expr>, P<Block>, Option<Ident>),
1016+
WhileLet(P<Pat>, P<Expr>, P<Block>, Option<SpannedIdent>),
10171017
/// A for loop, with an optional label
10181018
///
10191019
/// `'label: for pat in expr { block }`
10201020
///
10211021
/// This is desugared to a combination of `loop` and `match` expressions.
1022-
ForLoop(P<Pat>, P<Expr>, P<Block>, Option<Ident>),
1022+
ForLoop(P<Pat>, P<Expr>, P<Block>, Option<SpannedIdent>),
10231023
/// Conditionless loop (can be exited with break, continue, or return)
10241024
///
10251025
/// `'label: loop { block }`
1026-
Loop(P<Block>, Option<Ident>),
1026+
Loop(P<Block>, Option<SpannedIdent>),
10271027
/// A `match` block.
10281028
Match(P<Expr>, Vec<Arm>),
10291029
/// A closure (for example, `move |a, b, c| {a + b + c}`)

‎src/libsyntax/ext/expand.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// except according to those terms.
1010

1111
use ast::{Block, Crate, DeclKind, PatKind};
12-
use ast::{Local, Ident, Mac_, Name};
12+
use ast::{Local, Ident, Mac_, Name, SpannedIdent};
1313
use ast::{MacStmtStyle, Mrk, Stmt, StmtKind, ItemKind};
1414
use ast::TokenTree;
1515
use ast;
@@ -334,20 +334,20 @@ fn expand_mac_invoc<T>(mac: ast::Mac, ident: Option<Ident>, attrs: Vec<ast::Attr
334334
/// body is in a block enclosed by loop head so the renaming of loop label
335335
/// must be propagated to the enclosed context.
336336
fn expand_loop_block(loop_block: P<Block>,
337-
opt_ident: Option<Ident>,
338-
fld: &mut MacroExpander) -> (P<Block>, Option<Ident>) {
337+
opt_ident: Option<SpannedIdent>,
338+
fld: &mut MacroExpander) -> (P<Block>, Option<SpannedIdent>) {
339339
match opt_ident {
340340
Some(label) => {
341-
let new_label = fresh_name(label);
342-
let rename = (label, new_label);
341+
let new_label = fresh_name(label.node);
342+
let rename = (label.node, new_label);
343343

344344
// The rename *must not* be added to the pending list of current
345345
// syntax context otherwise an unrelated `break` or `continue` in
346346
// the same context will pick that up in the deferred renaming pass
347347
// and be renamed incorrectly.
348348
let mut rename_list = vec!(rename);
349349
let mut rename_fld = IdentRenamer{renames: &mut rename_list};
350-
let renamed_ident = rename_fld.fold_ident(label);
350+
let renamed_ident = rename_fld.fold_ident(label.node);
351351

352352
// The rename *must* be added to the enclosed syntax context for
353353
// `break` or `continue` to pick up because by definition they are
@@ -357,7 +357,7 @@ fn expand_loop_block(loop_block: P<Block>,
357357
let expanded_block = expand_block_elts(loop_block, fld);
358358
fld.cx.syntax_env.pop_frame();
359359

360-
(expanded_block, Some(renamed_ident))
360+
(expanded_block, Some(Spanned { node: renamed_ident, span: label.span }))
361361
}
362362
None => (fld.fold_block(loop_block), opt_ident)
363363
}

‎src/libsyntax/fold.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,23 +1212,27 @@ pub fn noop_fold_expr<T: Folder>(Expr {id, node, span, attrs}: Expr, folder: &mu
12121212
ExprKind::While(cond, body, opt_ident) => {
12131213
ExprKind::While(folder.fold_expr(cond),
12141214
folder.fold_block(body),
1215-
opt_ident.map(|i| folder.fold_ident(i)))
1215+
opt_ident.map(|label| respan(folder.new_span(label.span),
1216+
folder.fold_ident(label.node))))
12161217
}
12171218
ExprKind::WhileLet(pat, expr, body, opt_ident) => {
12181219
ExprKind::WhileLet(folder.fold_pat(pat),
12191220
folder.fold_expr(expr),
12201221
folder.fold_block(body),
1221-
opt_ident.map(|i| folder.fold_ident(i)))
1222+
opt_ident.map(|label| respan(folder.new_span(label.span),
1223+
folder.fold_ident(label.node))))
12221224
}
12231225
ExprKind::ForLoop(pat, iter, body, opt_ident) => {
12241226
ExprKind::ForLoop(folder.fold_pat(pat),
12251227
folder.fold_expr(iter),
12261228
folder.fold_block(body),
1227-
opt_ident.map(|i| folder.fold_ident(i)))
1229+
opt_ident.map(|label| respan(folder.new_span(label.span),
1230+
folder.fold_ident(label.node))))
12281231
}
12291232
ExprKind::Loop(body, opt_ident) => {
12301233
ExprKind::Loop(folder.fold_block(body),
1231-
opt_ident.map(|i| folder.fold_ident(i)))
1234+
opt_ident.map(|label| respan(folder.new_span(label.span),
1235+
folder.fold_ident(label.node))))
12321236
}
12331237
ExprKind::Match(expr, arms) => {
12341238
ExprKind::Match(folder.fold_expr(expr),

0 commit comments

Comments
 (0)