From d8074e65b02812e96cd5ed987795f5e8cfcef78d Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 2 Sep 2015 22:29:41 +0300 Subject: [PATCH 1/4] Use proper span for break and continue labels Fixes #28109 --- src/librustc/middle/cfg/construct.rs | 4 ++-- src/librustc/middle/liveness.rs | 4 ++-- src/librustc_back/svh.rs | 4 ++-- src/librustc_resolve/lib.rs | 6 +++--- src/librustc_trans/trans/expr.rs | 4 ++-- src/libsyntax/ast.rs | 4 ++-- src/libsyntax/fold.rs | 10 ++++++++-- src/libsyntax/parse/parser.rs | 15 +++++++++------ src/libsyntax/print/pprust.rs | 4 ++-- 9 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index 4d79867e5d2..3f6385ad82c 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -284,7 +284,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { } hir::ExprBreak(label) => { - let loop_scope = self.find_scope(expr, label); + let loop_scope = self.find_scope(expr, label.map(|l| l.node)); let b = self.add_ast_node(expr.id, &[pred]); self.add_exiting_edge(expr, b, loop_scope, loop_scope.break_index); @@ -292,7 +292,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { } hir::ExprAgain(label) => { - let loop_scope = self.find_scope(expr, label); + let loop_scope = self.find_scope(expr, label.map(|l| l.node)); let a = self.add_ast_node(expr.id, &[pred]); self.add_exiting_edge(expr, a, loop_scope, loop_scope.continue_index); diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 476df77e45b..561760b29f1 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -1049,7 +1049,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { hir::ExprBreak(opt_label) => { // Find which label this break jumps to - let sc = self.find_loop_scope(opt_label, expr.id, expr.span); + let sc = self.find_loop_scope(opt_label.map(|l| l.node), expr.id, expr.span); // Now that we know the label we're going to, // look it up in the break loop nodes table @@ -1063,7 +1063,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { hir::ExprAgain(opt_label) => { // Find which label this expr continues to - let sc = self.find_loop_scope(opt_label, expr.id, expr.span); + let sc = self.find_loop_scope(opt_label.map(|l| l.node), expr.id, expr.span); // Now that we know the label we're going to, // look it up in the continue loop nodes table diff --git a/src/librustc_back/svh.rs b/src/librustc_back/svh.rs index 9f527341bcd..5f6bf939fb2 100644 --- a/src/librustc_back/svh.rs +++ b/src/librustc_back/svh.rs @@ -277,8 +277,8 @@ mod svh_visitor { ExprRange(..) => SawExprRange, ExprPath(ref qself, _) => SawExprPath(qself.as_ref().map(|q| q.position)), ExprAddrOf(m, _) => SawExprAddrOf(m), - ExprBreak(id) => SawExprBreak(id.map(|id| id.name.as_str())), - ExprAgain(id) => SawExprAgain(id.map(|id| id.name.as_str())), + ExprBreak(id) => SawExprBreak(id.map(|id| id.node.name.as_str())), + ExprAgain(id) => SawExprAgain(id.map(|id| id.node.name.as_str())), ExprRet(..) => SawExprRet, ExprInlineAsm(ref asm) => SawExprInlineAsm(asm), ExprStruct(..) => SawExprStruct, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fee7d4d1d44..da9bae62d04 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3759,12 +3759,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } ExprBreak(Some(label)) | ExprAgain(Some(label)) => { - let renamed = mtwt::resolve(label); + let renamed = mtwt::resolve(label.node); match self.search_label(renamed) { None => { resolve_error(self, - expr.span, - ResolutionError::UndeclaredLabel(&label.name.as_str())) + label.span, + ResolutionError::UndeclaredLabel(&label.node.name.as_str())) } Some(DlDef(def @ DefLabel(_))) => { // Since this def is a label, it is never read. diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 730ab22e1e0..83644beae22 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -937,10 +937,10 @@ fn trans_rvalue_stmt_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, trans_into(bcx, &**e, Ignore) } hir::ExprBreak(label_opt) => { - controlflow::trans_break(bcx, expr, label_opt) + controlflow::trans_break(bcx, expr, label_opt.map(|l| l.node)) } hir::ExprAgain(label_opt) => { - controlflow::trans_cont(bcx, expr, label_opt) + controlflow::trans_cont(bcx, expr, label_opt.map(|l| l.node)) } hir::ExprRet(ref ex) => { // Check to see if the return expression itself is reachable. diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 25a3540c743..049d45b6e9e 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -891,9 +891,9 @@ pub enum Expr_ { /// A referencing operation (`&a` or `&mut a`) ExprAddrOf(Mutability, P), /// A `break`, with an optional label to break - ExprBreak(Option), + ExprBreak(Option), /// A `continue`, with an optional label - ExprAgain(Option), + ExprAgain(Option), /// A `return`, with an optional value to be returned ExprRet(Option>), diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index a547e537c9d..0cfddc9857c 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1299,8 +1299,14 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> }); ExprPath(qself, folder.fold_path(path)) } - ExprBreak(opt_ident) => ExprBreak(opt_ident.map(|x| folder.fold_ident(x))), - ExprAgain(opt_ident) => ExprAgain(opt_ident.map(|x| folder.fold_ident(x))), + ExprBreak(opt_ident) => ExprBreak(opt_ident.map(|label| + respan(folder.new_span(label.span), + folder.fold_ident(label.node))) + ), + ExprAgain(opt_ident) => ExprAgain(opt_ident.map(|label| + respan(folder.new_span(label.span), + folder.fold_ident(label.node))) + ), ExprRet(e) => ExprRet(e.map(|x| folder.fold_expr(x))), ExprInlineAsm(InlineAsm { inputs, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 0772d124db8..33f784b72bb 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2143,9 +2143,12 @@ impl<'a> Parser<'a> { } if try!(self.eat_keyword(keywords::Continue) ){ let ex = if self.token.is_lifetime() { - let lifetime = self.get_lifetime(); + let ex = ExprAgain(Some(Spanned{ + node: self.get_lifetime(), + span: self.span + })); try!(self.bump()); - ExprAgain(Some(lifetime)) + ex } else { ExprAgain(None) }; @@ -2161,7 +2164,6 @@ impl<'a> Parser<'a> { UnsafeBlock(ast::UserProvided)); } if try!(self.eat_keyword(keywords::Return) ){ - // RETURN expression if self.token.can_begin_expr() { let e = try!(self.parse_expr_nopanic()); hi = e.span.hi; @@ -2170,11 +2172,12 @@ impl<'a> Parser<'a> { ex = ExprRet(None); } } else if try!(self.eat_keyword(keywords::Break) ){ - // BREAK expression if self.token.is_lifetime() { - let lifetime = self.get_lifetime(); + ex = ExprBreak(Some(Spanned { + node: self.get_lifetime(), + span: self.span + })); try!(self.bump()); - ex = ExprBreak(Some(lifetime)); } else { ex = ExprBreak(None); } diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index b93a244df13..1dd43bb19cc 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1911,7 +1911,7 @@ impl<'a> State<'a> { try!(word(&mut self.s, "break")); try!(space(&mut self.s)); if let Some(ident) = opt_ident { - try!(self.print_ident(ident)); + try!(self.print_ident(ident.node)); try!(space(&mut self.s)); } } @@ -1919,7 +1919,7 @@ impl<'a> State<'a> { try!(word(&mut self.s, "continue")); try!(space(&mut self.s)); if let Some(ident) = opt_ident { - try!(self.print_ident(ident)); + try!(self.print_ident(ident.node)); try!(space(&mut self.s)) } } From f6244f1516d4466994b3f0eb3587ba676203f4ca Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 2 Sep 2015 22:35:04 +0300 Subject: [PATCH 2/4] Fix #28105 test and add a test for #28109 --- src/test/compile-fail/issue-28105.rs | 15 +++++---------- src/test/compile-fail/issue-28109.rs | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 10 deletions(-) create mode 100644 src/test/compile-fail/issue-28109.rs diff --git a/src/test/compile-fail/issue-28105.rs b/src/test/compile-fail/issue-28105.rs index 6ae635877b7..8e58d1aaf24 100644 --- a/src/test/compile-fail/issue-28105.rs +++ b/src/test/compile-fail/issue-28105.rs @@ -1,4 +1,4 @@ -// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT +// Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -11,13 +11,8 @@ // Make sure that a continue span actually contains the keyword. fn main() { - 'a: loop { - if false { - continue //~ ERROR use of undeclared label - 'b; - } else { - break //~ ERROR use of undeclared label - 'c; - } - } + continue //~ ERROR `continue` outside of loop + ; + break //~ ERROR `break` outside of loop + ; } diff --git a/src/test/compile-fail/issue-28109.rs b/src/test/compile-fail/issue-28109.rs new file mode 100644 index 00000000000..73163caa455 --- /dev/null +++ b/src/test/compile-fail/issue-28109.rs @@ -0,0 +1,20 @@ +// Copyright 2012-2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Make sure that label for continue and break is spanned correctly + +fn main() { + continue + 'b //~ ERROR use of undeclared label + ; + break + 'c //~ ERROR use of undeclared label + ; +} From 1a6934840edcef6bb37171f7d6d035ab59f5d725 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Thu, 3 Sep 2015 00:03:38 +0300 Subject: [PATCH 3/4] Fix hygienic-label-x tests --- src/test/compile-fail/hygienic-label-1.rs | 4 ++-- src/test/compile-fail/hygienic-label-3.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/compile-fail/hygienic-label-1.rs b/src/test/compile-fail/hygienic-label-1.rs index dd6682a6f42..305b43402da 100644 --- a/src/test/compile-fail/hygienic-label-1.rs +++ b/src/test/compile-fail/hygienic-label-1.rs @@ -9,9 +9,9 @@ // except according to those terms. macro_rules! foo { - () => { break 'x; } + () => { break 'x; } //~ ERROR use of undeclared label `'x` } pub fn main() { - 'x: loop { foo!() } //~ ERROR use of undeclared label `'x` + 'x: loop { foo!() } } diff --git a/src/test/compile-fail/hygienic-label-3.rs b/src/test/compile-fail/hygienic-label-3.rs index e58cd09a84c..b107b71024d 100644 --- a/src/test/compile-fail/hygienic-label-3.rs +++ b/src/test/compile-fail/hygienic-label-3.rs @@ -9,11 +9,11 @@ // except according to those terms. macro_rules! foo { - () => { break 'x; } + () => { break 'x; } //~ ERROR use of undeclared label `'x` } pub fn main() { 'x: for _ in 0..1 { - foo!() //~ ERROR use of undeclared label `'x` + foo!() }; } From c493084ec1b35b7ab59aa0878354bca10a610360 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Thu, 3 Sep 2015 11:54:17 +0300 Subject: [PATCH 4/4] Adapt the PR for HIR changes --- src/librustc_front/fold.rs | 10 ++++++++-- src/librustc_front/hir.rs | 4 ++-- src/librustc_front/print/pprust.rs | 4 ++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/librustc_front/fold.rs b/src/librustc_front/fold.rs index c663ebbf945..ba20b46090a 100644 --- a/src/librustc_front/fold.rs +++ b/src/librustc_front/fold.rs @@ -1124,8 +1124,14 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> }); ExprPath(qself, folder.fold_path(path)) } - ExprBreak(opt_ident) => ExprBreak(opt_ident.map(|x| folder.fold_ident(x))), - ExprAgain(opt_ident) => ExprAgain(opt_ident.map(|x| folder.fold_ident(x))), + ExprBreak(opt_ident) => ExprBreak(opt_ident.map(|label| + respan(folder.new_span(label.span), + folder.fold_ident(label.node))) + ), + ExprAgain(opt_ident) => ExprAgain(opt_ident.map(|label| + respan(folder.new_span(label.span), + folder.fold_ident(label.node))) + ), ExprRet(e) => ExprRet(e.map(|x| folder.fold_expr(x))), ExprInlineAsm(InlineAsm { inputs, diff --git a/src/librustc_front/hir.rs b/src/librustc_front/hir.rs index 6d7bef32ff7..4a53a4c60cf 100644 --- a/src/librustc_front/hir.rs +++ b/src/librustc_front/hir.rs @@ -730,9 +730,9 @@ pub enum Expr_ { /// A referencing operation (`&a` or `&mut a`) ExprAddrOf(Mutability, P), /// A `break`, with an optional label to break - ExprBreak(Option), + ExprBreak(Option), /// A `continue`, with an optional label - ExprAgain(Option), + ExprAgain(Option), /// A `return`, with an optional value to be returned ExprRet(Option>), diff --git a/src/librustc_front/print/pprust.rs b/src/librustc_front/print/pprust.rs index eeb1f2e167a..2f106f6a9e1 100644 --- a/src/librustc_front/print/pprust.rs +++ b/src/librustc_front/print/pprust.rs @@ -1587,7 +1587,7 @@ impl<'a> State<'a> { try!(word(&mut self.s, "break")); try!(space(&mut self.s)); if let Some(ident) = opt_ident { - try!(self.print_ident(ident)); + try!(self.print_ident(ident.node)); try!(space(&mut self.s)); } } @@ -1595,7 +1595,7 @@ impl<'a> State<'a> { try!(word(&mut self.s, "continue")); try!(space(&mut self.s)); if let Some(ident) = opt_ident { - try!(self.print_ident(ident)); + try!(self.print_ident(ident.node)); try!(space(&mut self.s)) } }