From 5205e2f8b8094a5f3d0205593820391dfe481808 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 15 Feb 2017 14:52:27 -0800 Subject: [PATCH 1/5] Normalize labeled and unlabeled breaks --- src/librustc/cfg/construct.rs | 15 +-- src/librustc/hir/intravisit.rs | 20 +-- src/librustc/hir/lowering.rs | 115 ++++++++++++------ src/librustc/hir/mod.rs | 9 +- src/librustc/hir/print.rs | 12 +- src/librustc/middle/liveness.rs | 25 +--- .../calculate_svh/svh_visitor.rs | 6 +- src/librustc_mir/build/scope.rs | 22 ++-- src/librustc_mir/hair/cx/expr.rs | 4 +- src/librustc_mir/hair/mod.rs | 4 +- src/librustc_passes/loops.rs | 23 ++-- src/librustc_typeck/check/mod.rs | 14 +-- src/test/run-pass/loop-break-value.rs | 8 -- 13 files changed, 138 insertions(+), 139 deletions(-) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index 595059332895d..bc3da13ccf82d 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -579,17 +579,12 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { fn find_scope(&self, expr: &hir::Expr, - label: Option) -> LoopScope { - match label { - None => *self.loop_scopes.last().unwrap(), - Some(label) => { - for l in &self.loop_scopes { - if l.loop_id == label.loop_id { - return *l; - } - } - span_bug!(expr.span, "no loop scope for id {}", label.loop_id); + label: hir::Label) -> LoopScope { + for l in &self.loop_scopes { + if l.loop_id == label.loop_id { + return *l; } } + span_bug!(expr.span, "no loop scope for id {}", label.loop_id); } } diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 1df6761506993..03b59aaf319c6 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -1006,18 +1006,18 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { ExprPath(ref qpath) => { visitor.visit_qpath(qpath, expression.id, expression.span); } - ExprBreak(None, ref opt_expr) => { + ExprBreak(label, ref opt_expr) => { + label.ident.map(|ident| { + visitor.visit_def_mention(Def::Label(label.loop_id)); + visitor.visit_name(ident.span, ident.node.name); + }); walk_list!(visitor, visit_expr, opt_expr); } - ExprBreak(Some(label), ref opt_expr) => { - visitor.visit_def_mention(Def::Label(label.loop_id)); - visitor.visit_name(label.span, label.name); - walk_list!(visitor, visit_expr, opt_expr); - } - ExprAgain(None) => {} - ExprAgain(Some(label)) => { - visitor.visit_def_mention(Def::Label(label.loop_id)); - visitor.visit_name(label.span, label.name); + ExprAgain(label) => { + label.ident.map(|ident| { + visitor.visit_def_mention(Def::Label(label.loop_id)); + visitor.visit_name(ident.span, ident.node.name); + }); } ExprRet(ref optional_expression) => { walk_list!(visitor, visit_expr, optional_expression); diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 8a4acb3d03880..d6f99327d4fa2 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -50,6 +50,7 @@ use util::nodemap::{DefIdMap, NodeMap, FxHashMap}; use std::collections::BTreeMap; use std::iter; +use std::mem; use syntax::attr; use syntax::ast::*; @@ -79,6 +80,8 @@ pub struct LoweringContext<'a> { impl_items: BTreeMap, bodies: FxHashMap, + loop_scopes: Vec, + type_def_lifetime_params: DefIdMap, } @@ -112,6 +115,7 @@ pub fn lower_crate(sess: &Session, trait_items: BTreeMap::new(), impl_items: BTreeMap::new(), bodies: FxHashMap(), + loop_scopes: Vec::new(), type_def_lifetime_params: DefIdMap(), }.lower_crate(krate) } @@ -244,6 +248,27 @@ impl<'a> LoweringContext<'a> { span } + fn with_loop_scope(&mut self, loop_id: NodeId, f: F) -> T + where F: FnOnce(&mut LoweringContext) -> T + { + let len = self.loop_scopes.len(); + self.loop_scopes.push(loop_id); + let result = f(self); + assert_eq!(len + 1, self.loop_scopes.len(), + "Loop scopes should be added and removed in stack order"); + self.loop_scopes.pop().unwrap(); + result + } + + fn with_new_loop_scopes(&mut self, f: F) -> T + where F: FnOnce(&mut LoweringContext) -> T + { + let loop_scopes = mem::replace(&mut self.loop_scopes, Vec::new()); + let result = f(self); + mem::replace(&mut self.loop_scopes, loop_scopes); + result + } + fn with_parent_def(&mut self, parent_id: NodeId, f: F) -> T where F: FnOnce(&mut LoweringContext) -> T { @@ -271,17 +296,23 @@ impl<'a> LoweringContext<'a> { o_id.map(|sp_ident| respan(sp_ident.span, sp_ident.node.name)) } - fn lower_label(&mut self, id: NodeId, label: Option>) -> Option { - label.map(|sp_ident| { - hir::Label { - span: sp_ident.span, - name: sp_ident.node.name, + fn lower_label(&mut self, label: Option<(NodeId, Spanned)>) -> hir::Label { + match label { + Some((id, label_ident)) => hir::Label { + ident: Some(label_ident), loop_id: match self.expect_full_def(id) { Def::Label(loop_id) => loop_id, _ => DUMMY_NODE_ID } + }, + None => hir::Label { + ident: None, + loop_id: match self.loop_scopes.last() { + Some(innermost_loop_id) => *innermost_loop_id, + _ => DUMMY_NODE_ID + } } - }) + } } fn lower_attrs(&mut self, attrs: &Vec) -> hir::HirVec { @@ -992,15 +1023,17 @@ impl<'a> LoweringContext<'a> { self.record_body(value, None)) } ItemKind::Fn(ref decl, unsafety, constness, abi, ref generics, ref body) => { - let body = self.lower_block(body); - let body = self.expr_block(body, ThinVec::new()); - let body_id = self.record_body(body, Some(decl)); - hir::ItemFn(self.lower_fn_decl(decl), - self.lower_unsafety(unsafety), - self.lower_constness(constness), - abi, - self.lower_generics(generics), - body_id) + self.with_new_loop_scopes(|this| { + let body = this.lower_block(body); + let body = this.expr_block(body, ThinVec::new()); + let body_id = this.record_body(body, Some(decl)); + hir::ItemFn(this.lower_fn_decl(decl), + this.lower_unsafety(unsafety), + this.lower_constness(constness), + abi, + this.lower_generics(generics), + body_id) + }) } ItemKind::Mod(ref m) => hir::ItemMod(self.lower_mod(m)), ItemKind::ForeignMod(ref nm) => hir::ItemForeignMod(self.lower_foreign_mod(nm)), @@ -1562,13 +1595,17 @@ impl<'a> LoweringContext<'a> { hir::ExprIf(P(self.lower_expr(cond)), self.lower_block(blk), else_opt) } ExprKind::While(ref cond, ref body, opt_ident) => { - hir::ExprWhile(P(self.lower_expr(cond)), self.lower_block(body), - self.lower_opt_sp_ident(opt_ident)) + self.with_loop_scope(e.id, |this| + hir::ExprWhile( + P(this.lower_expr(cond)), + this.lower_block(body), + this.lower_opt_sp_ident(opt_ident))) } ExprKind::Loop(ref body, opt_ident) => { - hir::ExprLoop(self.lower_block(body), - self.lower_opt_sp_ident(opt_ident), - hir::LoopSource::Loop) + self.with_loop_scope(e.id, |this| + hir::ExprLoop(this.lower_block(body), + this.lower_opt_sp_ident(opt_ident), + hir::LoopSource::Loop)) } ExprKind::Match(ref expr, ref arms) => { hir::ExprMatch(P(self.lower_expr(expr)), @@ -1576,12 +1613,14 @@ impl<'a> LoweringContext<'a> { hir::MatchSource::Normal) } ExprKind::Closure(capture_clause, ref decl, ref body, fn_decl_span) => { - self.with_parent_def(e.id, |this| { - let expr = this.lower_expr(body); - hir::ExprClosure(this.lower_capture_clause(capture_clause), - this.lower_fn_decl(decl), - this.record_body(expr, Some(decl)), - fn_decl_span) + self.with_new_loop_scopes(|this| { + this.with_parent_def(e.id, |this| { + let expr = this.lower_expr(body); + hir::ExprClosure(this.lower_capture_clause(capture_clause), + this.lower_fn_decl(decl), + this.record_body(expr, Some(decl)), + fn_decl_span) + }) }) } ExprKind::Block(ref blk) => hir::ExprBlock(self.lower_block(blk)), @@ -1660,10 +1699,13 @@ impl<'a> LoweringContext<'a> { hir::ExprPath(self.lower_qpath(e.id, qself, path, ParamMode::Optional)) } ExprKind::Break(opt_ident, ref opt_expr) => { - hir::ExprBreak(self.lower_label(e.id, opt_ident), + hir::ExprBreak( + self.lower_label(opt_ident.map(|ident| (e.id, ident))), opt_expr.as_ref().map(|x| P(self.lower_expr(x)))) } - ExprKind::Continue(opt_ident) => hir::ExprAgain(self.lower_label(e.id, opt_ident)), + ExprKind::Continue(opt_ident) => + hir::ExprAgain( + self.lower_label(opt_ident.map(|ident| (e.id, ident)))), ExprKind::Ret(ref e) => hir::ExprRet(e.as_ref().map(|x| P(self.lower_expr(x)))), ExprKind::InlineAsm(ref asm) => { let hir_asm = hir::InlineAsm { @@ -1804,9 +1846,14 @@ impl<'a> LoweringContext<'a> { // } // } + let (body, break_expr, sub_expr) = self.with_loop_scope(e.id, |this| ( + this.lower_block(body), + this.expr_break(e.span, ThinVec::new()), + P(this.lower_expr(sub_expr)), + )); + // ` => ` let pat_arm = { - let body = self.lower_block(body); let body_expr = P(self.expr_block(body, ThinVec::new())); let pat = self.lower_pat(pat); self.arm(hir_vec![pat], body_expr) @@ -1815,13 +1862,11 @@ impl<'a> LoweringContext<'a> { // `_ => break` let break_arm = { let pat_under = self.pat_wild(e.span); - let break_expr = self.expr_break(e.span, ThinVec::new()); self.arm(hir_vec![pat_under], break_expr) }; // `match { ... }` let arms = hir_vec![pat_arm, break_arm]; - let sub_expr = P(self.lower_expr(sub_expr)); let match_expr = self.expr(e.span, hir::ExprMatch(sub_expr, arms, @@ -1863,7 +1908,7 @@ impl<'a> LoweringContext<'a> { // `::std::option::Option::Some() => ` let pat_arm = { - let body_block = self.lower_block(body); + let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body)); let body_expr = P(self.expr_block(body_block, ThinVec::new())); let pat = self.lower_pat(pat); let some_pat = self.pat_some(e.span, pat); @@ -1873,7 +1918,8 @@ impl<'a> LoweringContext<'a> { // `::std::option::Option::None => break` let break_arm = { - let break_expr = self.expr_break(e.span, ThinVec::new()); + let break_expr = self.with_loop_scope(e.id, |this| + this.expr_break(e.span, ThinVec::new())); let pat = self.pat_none(e.span); self.arm(hir_vec![pat], break_expr) }; @@ -2151,7 +2197,8 @@ impl<'a> LoweringContext<'a> { } fn expr_break(&mut self, span: Span, attrs: ThinVec) -> P { - P(self.expr(span, hir::ExprBreak(None, None), attrs)) + let expr_break = hir::ExprBreak(self.lower_label(None), None); + P(self.expr(span, expr_break, attrs)) } fn expr_call(&mut self, span: Span, e: P, args: hir::HirVec) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 4ebe416e1bfe6..fc5282d1af2f2 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -36,7 +36,7 @@ use util::nodemap::{NodeMap, FxHashMap, FxHashSet}; use syntax_pos::{Span, ExpnId, DUMMY_SP}; use syntax::codemap::{self, Spanned}; use syntax::abi::Abi; -use syntax::ast::{Name, NodeId, DUMMY_NODE_ID, AsmDialect}; +use syntax::ast::{Ident, Name, NodeId, DUMMY_NODE_ID, AsmDialect}; use syntax::ast::{Attribute, Lit, StrStyle, FloatTy, IntTy, UintTy, MetaItem}; use syntax::ptr::P; use syntax::symbol::{Symbol, keywords}; @@ -959,9 +959,9 @@ pub enum Expr_ { /// A referencing operation (`&a` or `&mut a`) ExprAddrOf(Mutability, P), /// A `break`, with an optional label to break - ExprBreak(Option