From 93edb7c17bd4d2d56b0f1f510de5f820cf61bb5f Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 5 Feb 2015 18:26:58 +0100 Subject: [PATCH] debuginfo: Fix problem with debug locations of constants in match patterns. --- src/librustc/middle/check_match.rs | 65 ++++++++++--- src/librustc/middle/const_eval.rs | 17 ++-- src/librustc_trans/trans/_match.rs | 45 +++++++-- .../debuginfo/constant-in-match-pattern.rs | 92 +++++++++++++++++++ 4 files changed, 189 insertions(+), 30 deletions(-) create mode 100644 src/test/debuginfo/constant-in-match-pattern.rs diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index 418cdf957183f..0d2af78ed0675 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -28,7 +28,7 @@ use std::iter::{range_inclusive, AdditiveIterator, FromIterator, repeat}; use std::num::Float; use std::slice; use syntax::ast::{self, DUMMY_NODE_ID, NodeId, Pat}; -use syntax::ast_util::walk_pat; +use syntax::ast_util; use syntax::codemap::{Span, Spanned, DUMMY_SP}; use syntax::fold::{Folder, noop_fold_pat}; use syntax::print::pprust::pat_to_string; @@ -36,6 +36,7 @@ use syntax::parse::token; use syntax::ptr::P; use syntax::visit::{self, Visitor, FnKind}; use util::ppaux::ty_to_string; +use util::nodemap::FnvHashMap; pub const DUMMY_WILD_PAT: &'static Pat = &Pat { id: DUMMY_NODE_ID, @@ -171,7 +172,7 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) { } } - let mut static_inliner = StaticInliner::new(cx.tcx); + let mut static_inliner = StaticInliner::new(cx.tcx, None); let inlined_arms = arms.iter().map(|arm| { (arm.pats.iter().map(|pat| { static_inliner.fold_pat((*pat).clone()) @@ -235,7 +236,7 @@ fn is_expr_const_nan(tcx: &ty::ctxt, expr: &ast::Expr) -> bool { } fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) { - walk_pat(pat, |p| { + ast_util::walk_pat(pat, |p| { match p.node { ast::PatIdent(ast::BindByValue(ast::MutImmutable), ident, None) => { let pat_ty = ty::pat_ty(cx.tcx, p); @@ -266,7 +267,7 @@ fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) // Check that we do not match against a static NaN (#6804) fn check_for_static_nan(cx: &MatchCheckCtxt, pat: &Pat) { - walk_pat(pat, |p| { + ast_util::walk_pat(pat, |p| { match p.node { ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => { span_warn!(cx.tcx.sess, p.span, E0003, @@ -399,28 +400,50 @@ fn const_val_to_expr(value: &const_val) -> P { pub struct StaticInliner<'a, 'tcx: 'a> { pub tcx: &'a ty::ctxt<'tcx>, - pub failed: bool + pub failed: bool, + pub renaming_map: Option<&'a mut FnvHashMap<(NodeId, Span), NodeId>>, } impl<'a, 'tcx> StaticInliner<'a, 'tcx> { - pub fn new<'b>(tcx: &'b ty::ctxt<'tcx>) -> StaticInliner<'b, 'tcx> { + pub fn new<'b>(tcx: &'b ty::ctxt<'tcx>, + renaming_map: Option<&'b mut FnvHashMap<(NodeId, Span), NodeId>>) + -> StaticInliner<'b, 'tcx> { StaticInliner { tcx: tcx, - failed: false + failed: false, + renaming_map: renaming_map } } } +struct RenamingRecorder<'map> { + substituted_node_id: NodeId, + origin_span: Span, + renaming_map: &'map mut FnvHashMap<(NodeId, Span), NodeId> +} + +impl<'map> ast_util::IdVisitingOperation for RenamingRecorder<'map> { + fn visit_id(&mut self, node_id: NodeId) { + let key = (node_id, self.origin_span); + self.renaming_map.insert(key, self.substituted_node_id); + } +} + impl<'a, 'tcx> Folder for StaticInliner<'a, 'tcx> { fn fold_pat(&mut self, pat: P) -> P { - match pat.node { + return match pat.node { ast::PatIdent(..) | ast::PatEnum(..) => { let def = self.tcx.def_map.borrow().get(&pat.id).cloned(); match def { Some(DefConst(did)) => match lookup_const_by_id(self.tcx, did) { Some(const_expr) => { - const_expr_to_pat(self.tcx, const_expr).map(|mut new_pat| { - new_pat.span = pat.span; + const_expr_to_pat(self.tcx, const_expr, pat.span).map(|new_pat| { + + if let Some(ref mut renaming_map) = self.renaming_map { + // Record any renamings we do here + record_renamings(const_expr, &pat, renaming_map); + } + new_pat }) } @@ -435,6 +458,24 @@ impl<'a, 'tcx> Folder for StaticInliner<'a, 'tcx> { } } _ => noop_fold_pat(pat, self) + }; + + fn record_renamings(const_expr: &ast::Expr, + substituted_pat: &ast::Pat, + renaming_map: &mut FnvHashMap<(NodeId, Span), NodeId>) { + let mut renaming_recorder = RenamingRecorder { + substituted_node_id: substituted_pat.id, + origin_span: substituted_pat.span, + renaming_map: renaming_map, + }; + + let mut id_visitor = ast_util::IdVisitor { + operation: &mut renaming_recorder, + pass_through_items: true, + visited_outermost: false, + }; + + id_visitor.visit_expr(const_expr); } } } @@ -953,7 +994,7 @@ fn check_local(cx: &mut MatchCheckCtxt, loc: &ast::Local) { ast::LocalFor => "`for` loop" }; - let mut static_inliner = StaticInliner::new(cx.tcx); + let mut static_inliner = StaticInliner::new(cx.tcx, None); is_refutable(cx, &*static_inliner.fold_pat(loc.pat.clone()), |pat| { span_err!(cx.tcx.sess, loc.pat.span, E0005, "refutable pattern in {} binding: `{}` not covered", @@ -1040,7 +1081,7 @@ fn check_legality_of_move_bindings(cx: &MatchCheckCtxt, }; for pat in pats { - walk_pat(&**pat, |p| { + ast_util::walk_pat(&**pat, |p| { if pat_is_binding(def_map, &*p) { match p.node { ast::PatIdent(ast::BindByValue(_), _, ref sub) => { diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index fa5d5227be535..2d764518a47b2 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -22,6 +22,7 @@ use middle::astconv_util::{ast_ty_to_prim_ty}; use util::nodemap::DefIdMap; use syntax::ast::{self, Expr}; +use syntax::codemap::Span; use syntax::parse::token::InternedString; use syntax::ptr::P; use syntax::visit::{self, Visitor}; @@ -304,10 +305,10 @@ pub enum const_val { const_bool(bool) } -pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P { +pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr, span: Span) -> P { let pat = match expr.node { ast::ExprTup(ref exprs) => - ast::PatTup(exprs.iter().map(|expr| const_expr_to_pat(tcx, &**expr)).collect()), + ast::PatTup(exprs.iter().map(|expr| const_expr_to_pat(tcx, &**expr, span)).collect()), ast::ExprCall(ref callee, ref args) => { let def = tcx.def_map.borrow()[callee.id].clone(); @@ -319,7 +320,7 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P { def::DefVariant(_, variant_did, _) => def_to_path(tcx, variant_did), _ => unreachable!() }; - let pats = args.iter().map(|expr| const_expr_to_pat(tcx, &**expr)).collect(); + let pats = args.iter().map(|expr| const_expr_to_pat(tcx, &**expr, span)).collect(); ast::PatEnum(path, Some(pats)) } @@ -328,7 +329,7 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P { span: codemap::DUMMY_SP, node: ast::FieldPat { ident: field.ident.node, - pat: const_expr_to_pat(tcx, &*field.expr), + pat: const_expr_to_pat(tcx, &*field.expr, span), is_shorthand: false, }, }).collect(); @@ -336,7 +337,7 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P { } ast::ExprVec(ref exprs) => { - let pats = exprs.iter().map(|expr| const_expr_to_pat(tcx, &**expr)).collect(); + let pats = exprs.iter().map(|expr| const_expr_to_pat(tcx, &**expr, span)).collect(); ast::PatVec(pats, None, vec![]) } @@ -349,7 +350,7 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P { ast::PatEnum(path.clone(), None), _ => { match lookup_const(tcx, expr) { - Some(actual) => return const_expr_to_pat(tcx, actual), + Some(actual) => return const_expr_to_pat(tcx, actual, span), _ => unreachable!() } } @@ -358,14 +359,14 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P { ast::ExprQPath(_) => { match lookup_const(tcx, expr) { - Some(actual) => return const_expr_to_pat(tcx, actual), + Some(actual) => return const_expr_to_pat(tcx, actual, span), _ => unreachable!() } } _ => ast::PatLit(P(expr.clone())) }; - P(ast::Pat { id: expr.id, node: pat, span: expr.span }) + P(ast::Pat { id: expr.id, node: pat, span: span }) } pub fn eval_const_expr(tcx: &ty::ctxt, e: &Expr) -> const_val { diff --git a/src/librustc_trans/trans/_match.rs b/src/librustc_trans/trans/_match.rs index feea8282f3995..610b6e1aa3807 100644 --- a/src/librustc_trans/trans/_match.rs +++ b/src/librustc_trans/trans/_match.rs @@ -213,7 +213,7 @@ use trans::expr::{self, Dest}; use trans::tvec; use trans::type_of; use middle::ty::{self, Ty}; -use session::config::FullDebugInfo; +use session::config::{NoDebugInfo, FullDebugInfo}; use util::common::indenter; use util::nodemap::FnvHashMap; use util::ppaux::{Repr, vec_map_to_string}; @@ -222,7 +222,7 @@ use std; use std::iter::AdditiveIterator; use std::rc::Rc; use syntax::ast; -use syntax::ast::{DUMMY_NODE_ID, Ident}; +use syntax::ast::{DUMMY_NODE_ID, Ident, NodeId}; use syntax::codemap::Span; use syntax::fold::Folder; use syntax::ptr::P; @@ -366,6 +366,9 @@ struct Match<'a, 'p: 'a, 'blk: 'a, 'tcx: 'blk> { pats: Vec<&'p ast::Pat>, data: &'a ArmData<'p, 'blk, 'tcx>, bound_ptrs: Vec<(Ident, ValueRef)>, + // Thread along renamings done by the check_match::StaticInliner, so we can + // map back to original NodeIds + pat_renaming_map: Option<&'a FnvHashMap<(NodeId, Span), NodeId>> } impl<'a, 'p, 'blk, 'tcx> Repr<'tcx> for Match<'a, 'p, 'blk, 'tcx> { @@ -419,7 +422,8 @@ fn expand_nested_bindings<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>, Match { pats: pats, data: &*br.data, - bound_ptrs: bound_ptrs + bound_ptrs: bound_ptrs, + pat_renaming_map: br.pat_renaming_map, } }).collect() } @@ -463,7 +467,8 @@ fn enter_match<'a, 'b, 'p, 'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>, Match { pats: pats, data: br.data, - bound_ptrs: bound_ptrs + bound_ptrs: bound_ptrs, + pat_renaming_map: br.pat_renaming_map, } }) }).collect() @@ -570,14 +575,23 @@ fn enter_opt<'a, 'p, 'blk, 'tcx>( // needs to be conditionally matched at runtime; for example, the discriminant // on a set of enum variants or a literal. fn get_branches<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>, - m: &[Match<'a, 'p, 'blk, 'tcx>], col: uint) + m: &[Match<'a, 'p, 'blk, 'tcx>], + col: uint) -> Vec> { let tcx = bcx.tcx(); let mut found: Vec = vec![]; for br in m { let cur = br.pats[col]; - let debug_loc = DebugLoc::At(cur.id, cur.span); + let debug_loc = match br.pat_renaming_map { + Some(pat_renaming_map) => { + match pat_renaming_map.get(&(cur.id, cur.span)) { + Some(&id) => DebugLoc::At(id, cur.span), + None => DebugLoc::At(cur.id, cur.span), + } + } + None => DebugLoc::None + }; let opt = match cur.node { ast::PatLit(ref l) => { @@ -1419,16 +1433,27 @@ fn trans_match_inner<'blk, 'tcx>(scope_cx: Block<'blk, 'tcx>, bindings_map: create_bindings_map(bcx, &*arm.pats[0], discr_expr, &*arm.body) }).collect(); - let mut static_inliner = StaticInliner::new(scope_cx.tcx()); - let arm_pats: Vec>> = arm_datas.iter().map(|arm_data| { - arm_data.arm.pats.iter().map(|p| static_inliner.fold_pat((*p).clone())).collect() - }).collect(); + let mut pat_renaming_map = if scope_cx.sess().opts.debuginfo != NoDebugInfo { + Some(FnvHashMap()) + } else { + None + }; + + let arm_pats: Vec>> = { + let mut static_inliner = StaticInliner::new(scope_cx.tcx(), + pat_renaming_map.as_mut()); + arm_datas.iter().map(|arm_data| { + arm_data.arm.pats.iter().map(|p| static_inliner.fold_pat((*p).clone())).collect() + }).collect() + }; + let mut matches = Vec::new(); for (arm_data, pats) in arm_datas.iter().zip(arm_pats.iter()) { matches.extend(pats.iter().map(|p| Match { pats: vec![&**p], data: arm_data, bound_ptrs: Vec::new(), + pat_renaming_map: pat_renaming_map.as_ref() })); } diff --git a/src/test/debuginfo/constant-in-match-pattern.rs b/src/test/debuginfo/constant-in-match-pattern.rs new file mode 100644 index 0000000000000..487c69a85d6ac --- /dev/null +++ b/src/test/debuginfo/constant-in-match-pattern.rs @@ -0,0 +1,92 @@ +// Copyright 2013-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. + +// ignore-android: FIXME(#10381) +// min-lldb-version: 310 + +// compile-flags:-g + +#![allow(unused_variables)] +#![allow(dead_code)] +#![omit_gdb_pretty_printer_section] + +// This test makes sure that the compiler doesn't crash when trying to assign +// debug locations to 'constant' patterns in match expressions. + +const CONSTANT: u64 = 3; + +struct Struct { + a: isize, + b: usize, +} +const STRUCT: Struct = Struct { a: 1, b: 2 }; + +struct TupleStruct(u32); +const TUPLE_STRUCT: TupleStruct = TupleStruct(4); + +enum Enum { + Variant1(char), + Variant2 { a: u8 }, + Variant3 +} +const VARIANT1: Enum = Enum::Variant1('v'); +const VARIANT2: Enum = Enum::Variant2 { a: 2 }; +const VARIANT3: Enum = Enum::Variant3; + +const STRING: &'static str = "String"; + +fn main() { + + match 1 { + CONSTANT => {} + _ => {} + }; + + // if let 3 = CONSTANT {} + + match (Struct { a: 2, b: 2 }) { + STRUCT => {} + _ => {} + }; + + // if let STRUCT = STRUCT {} + + match TupleStruct(3) { + TUPLE_STRUCT => {} + _ => {} + }; + + // if let TupleStruct(4) = TUPLE_STRUCT {} + + match VARIANT3 { + VARIANT1 => {}, + VARIANT2 => {}, + VARIANT3 => {}, + _ => {} + }; + + match (VARIANT3, VARIANT2) { + (VARIANT1, VARIANT3) => {}, + (VARIANT2, VARIANT2) => {}, + (VARIANT3, VARIANT1) => {}, + _ => {} + }; + + // if let VARIANT1 = Enum::Variant3 {} + // if let VARIANT2 = Enum::Variant3 {} + // if let VARIANT3 = Enum::Variant3 {} + + match "abc" { + STRING => {}, + _ => {} + } + + if let STRING = "def" {} +}