Skip to content

Commit

Permalink
auto merge of #19357 : michaelwoerister/rust/fix-issue-18791, r=alexc…
Browse files Browse the repository at this point in the history
…richton

One negative side-effect of this change is that there might be quite a bit of copying strings out of the codemap, i.e. one copy for every block that gets translated, just for taking a look at the last character of the block. If this turns out to cause a performance problem then `CodeMap::span_to_snippet()` could be changed return `Option<&str>` instead of `Option<String>`.

Fixes #18791
  • Loading branch information
bors committed Dec 2, 2014
2 parents 8dbe632 + 61a0a7f commit 2b35e6f
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/librustc_trans/trans/base.rs
Expand Up @@ -1816,7 +1816,7 @@ pub fn trans_closure<'a, 'b, 'tcx>(ccx: &CrateContext<'a, 'tcx>,

// cleanup scope for the incoming arguments
let fn_cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(fn_ast_id, body.span, true);
debuginfo::get_cleanup_debug_loc_for_ast_node(ccx, fn_ast_id, body.span, true);
let arg_scope = fcx.push_custom_cleanup_scope_with_debug_loc(fn_cleanup_debug_loc);

let block_ty = node_id_type(bcx, body.id);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_trans/trans/controlflow.rs
Expand Up @@ -55,7 +55,7 @@ pub fn trans_stmt<'blk, 'tcx>(cx: Block<'blk, 'tcx>,

let id = ast_util::stmt_id(s);
let cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(id, s.span, false);
debuginfo::get_cleanup_debug_loc_for_ast_node(bcx.ccx(), id, s.span, false);
fcx.push_ast_cleanup_scope(cleanup_debug_loc);

match s.node {
Expand Down Expand Up @@ -103,7 +103,7 @@ pub fn trans_block<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
let mut bcx = bcx;

let cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(b.id, b.span, true);
debuginfo::get_cleanup_debug_loc_for_ast_node(bcx.ccx(), b.id, b.span, true);
fcx.push_ast_cleanup_scope(cleanup_debug_loc);

for s in b.stmts.iter() {
Expand Down
35 changes: 23 additions & 12 deletions src/librustc_trans/trans/debuginfo.rs
Expand Up @@ -1047,10 +1047,11 @@ pub fn create_argument_metadata(bcx: Block, arg: &ast::Arg) {
})
}

pub fn get_cleanup_debug_loc_for_ast_node(node_id: ast::NodeId,
node_span: Span,
is_block: bool)
-> NodeInfo {
pub fn get_cleanup_debug_loc_for_ast_node<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
node_id: ast::NodeId,
node_span: Span,
is_block: bool)
-> NodeInfo {
// A debug location needs two things:
// (1) A span (of which only the beginning will actually be used)
// (2) An AST node-id which will be used to look up the lexical scope
Expand Down Expand Up @@ -1080,15 +1081,25 @@ pub fn get_cleanup_debug_loc_for_ast_node(node_id: ast::NodeId,
// scope is actually left when the cleanup code is executed.
// In practice it shouldn't make much of a difference.

let cleanup_span = if is_block {
Span {
lo: node_span.hi - codemap::BytePos(1), // closing brace should always be 1 byte...
hi: node_span.hi,
expn_id: node_span.expn_id
let mut cleanup_span = node_span;

if is_block {
// Not all blocks actually have curly braces (e.g. simple closure
// bodies), in which case we also just want to return the span of the
// whole expression.
let code_snippet = cx.sess().codemap().span_to_snippet(node_span);
if let Some(code_snippet) = code_snippet {
let bytes = code_snippet.as_bytes();

if bytes.len() > 0 && bytes[bytes.len()-1 ..] == b"}" {
cleanup_span = Span {
lo: node_span.hi - codemap::BytePos(1),
hi: node_span.hi,
expn_id: node_span.expn_id
};
}
}
} else {
node_span
};
}

NodeInfo {
id: node_id,
Expand Down
11 changes: 8 additions & 3 deletions src/librustc_trans/trans/expr.rs
Expand Up @@ -97,7 +97,8 @@ pub fn trans_into<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,

debug!("trans_into() expr={}", expr.repr(bcx.tcx()));

let cleanup_debug_loc = debuginfo::get_cleanup_debug_loc_for_ast_node(expr.id,
let cleanup_debug_loc = debuginfo::get_cleanup_debug_loc_for_ast_node(bcx.ccx(),
expr.id,
expr.span,
false);
bcx.fcx.push_ast_cleanup_scope(cleanup_debug_loc);
Expand Down Expand Up @@ -130,7 +131,8 @@ pub fn trans<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
let mut bcx = bcx;
let fcx = bcx.fcx;

let cleanup_debug_loc = debuginfo::get_cleanup_debug_loc_for_ast_node(expr.id,
let cleanup_debug_loc = debuginfo::get_cleanup_debug_loc_for_ast_node(bcx.ccx(),
expr.id,
expr.span,
false);
fcx.push_ast_cleanup_scope(cleanup_debug_loc);
Expand Down Expand Up @@ -618,7 +620,10 @@ fn trans_datum_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
ast::ExprRepeat(..) | ast::ExprVec(..) => {
// Special case for slices.
let cleanup_debug_loc =
debuginfo::get_cleanup_debug_loc_for_ast_node(x.id, x.span, false);
debuginfo::get_cleanup_debug_loc_for_ast_node(bcx.ccx(),
x.id,
x.span,
false);
fcx.push_ast_cleanup_scope(cleanup_debug_loc);
let datum = unpack_datum!(
bcx, tvec::trans_slice_vec(bcx, expr, &**x));
Expand Down
28 changes: 28 additions & 0 deletions src/test/debuginfo/multi-byte-chars.rs
@@ -0,0 +1,28 @@
// Copyright 2013-2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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

#![feature(non_ascii_idents)]

// This test checks whether debuginfo generation can handle multi-byte UTF-8
// characters at the end of a block. There's no need to do anything in the
// debugger -- just make sure that the compiler doesn't crash.
// See also issue #18791.

struct C { θ: u8 }

fn main() {
let x = C { θ: 0 };
(|c: C| c.θ )(x);
}

0 comments on commit 2b35e6f

Please sign in to comment.