Skip to content

Get all (but one) of debuginfo tests to pass with MIR codegen. #32952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/librustc/mir/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ pub struct Mir<'tcx> {
/// through the resulting reference.
pub temp_decls: Vec<TempDecl<'tcx>>,

/// Names and capture modes of all the closure upvars, assuming
/// the first argument is either the closure or a reference to it.
pub upvar_decls: Vec<UpvarDecl>,

/// A span representing this MIR, for error reporting
pub span: Span,
}
Expand Down Expand Up @@ -197,7 +201,20 @@ pub struct ArgDecl<'tcx> {

/// If true, this argument is a tuple after monomorphization,
/// and has to be collected from multiple actual arguments.
pub spread: bool
pub spread: bool,

/// Either special_idents::invalid or the name of a single-binding
/// pattern associated with this argument. Useful for debuginfo.
pub debug_name: Name
}

/// A closure capture, with its name and mode.
#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
pub struct UpvarDecl {
pub debug_name: Name,

/// If true, the capture is behind a reference.
pub by_ref: bool
}

///////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ macro_rules! make_mir_visitor {
ref $($mutability)* var_decls,
ref $($mutability)* arg_decls,
ref $($mutability)* temp_decls,
upvar_decls: _,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to provide a visit_upvar_decl method, just for completeness.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want it to be part of MIR itself, really - initially I had upvar_names: Vec<Name> but realized that I also needed to know if they're by-ref, that's the only reason it's a struct.

ref $($mutability)* span,
} = *mir;

Expand Down Expand Up @@ -599,7 +600,8 @@ macro_rules! make_mir_visitor {
arg_decl: & $($mutability)* ArgDecl<'tcx>) {
let ArgDecl {
ref $($mutability)* ty,
spread: _
spread: _,
debug_name: _
} = *arg_decl;

self.visit_ty(ty);
Expand Down
39 changes: 23 additions & 16 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,28 @@ impl<'a,'tcx> Builder<'a,'tcx> {
-> BlockAnd<()> {
let discriminant_lvalue = unpack!(block = self.as_lvalue(block, discriminant));

// Before we do anything, create uninitialized variables with
// suitable extent for all of the bindings in this match. It's
// easiest to do this up front because some of these arms may
// be unreachable or reachable multiple times.
let var_scope_id = self.innermost_scope_id();
for arm in &arms {
self.declare_bindings(var_scope_id, &arm.patterns[0]);
}

let mut arm_blocks = ArmBlocks {
blocks: arms.iter()
.map(|_| self.cfg.start_new_block())
.collect(),
};

let arm_bodies: Vec<ExprRef<'tcx>> =
arms.iter()
.map(|arm| arm.body.clone())
.collect();
// Get the body expressions and their scopes, while declaring bindings.
let arm_bodies: Vec<_> = arms.iter().enumerate().map(|(i, arm)| {
// Assume that all expressions are wrapped in Scope.
let body = self.hir.mirror(arm.body.clone());
match body.kind {
ExprKind::Scope { extent, value } => {
let scope_id = self.push_scope(extent, arm_blocks.blocks[i]);
self.declare_bindings(scope_id, &arm.patterns[0]);
(extent, self.scopes.pop().unwrap(), value)
}
_ => {
span_bug!(body.span, "arm body is not wrapped in Scope {:?}",
body.kind);
}
}
}).collect();

// assemble a list of candidates: there is one candidate per
// pattern, which means there may be more than one candidate
Expand Down Expand Up @@ -95,11 +98,15 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// all the arm blocks will rejoin here
let end_block = self.cfg.start_new_block();

for (arm_index, arm_body) in arm_bodies.into_iter().enumerate() {
let scope_id = self.innermost_scope_id();
for (arm_index, (extent, scope, body)) in arm_bodies.into_iter().enumerate() {
let mut arm_block = arm_blocks.blocks[arm_index];
unpack!(arm_block = self.into(destination, arm_block, arm_body));
// Re-enter the scope we created the bindings in.
self.scopes.push(scope);
unpack!(arm_block = self.into(destination, arm_block, body));
unpack!(arm_block = self.pop_scope(extent, arm_block));
self.cfg.terminate(arm_block,
var_scope_id,
scope_id,
span,
TerminatorKind::Goto { target: end_block });
}
Expand Down
43 changes: 41 additions & 2 deletions src/librustc_mir/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@

use hair::cx::Cx;
use rustc::middle::region::{CodeExtent, CodeExtentData};
use rustc::ty::{FnOutput, Ty};
use rustc::ty::{self, FnOutput, Ty};
use rustc::mir::repr::*;
use rustc_data_structures::fnv::FnvHashMap;
use rustc::hir;
use rustc::hir::pat_util::pat_is_binding;
use std::ops::{Index, IndexMut};
use syntax::ast;
use syntax::codemap::Span;
use syntax::parse::token;

pub struct Builder<'a, 'tcx: 'a> {
hir: Cx<'a, 'tcx>,
Expand Down Expand Up @@ -224,13 +226,37 @@ pub fn construct<'a,'tcx>(hir: Cx<'a,'tcx>,
true
}));

// Gather the upvars of a closure, if any.
let upvar_decls: Vec<_> = tcx.with_freevars(fn_id, |freevars| {
freevars.iter().map(|fv| {
let by_ref = tcx.upvar_capture(ty::UpvarId {
var_id: fv.def.var_id(),
closure_expr_id: fn_id
}).map_or(false, |capture| match capture {
ty::UpvarCapture::ByValue => false,
ty::UpvarCapture::ByRef(..) => true
});
let mut decl = UpvarDecl {
debug_name: token::special_idents::invalid.name,
by_ref: by_ref
};
if let Some(hir::map::NodeLocal(pat)) = tcx.map.find(fv.def.var_id()) {
if let hir::PatKind::Ident(_, ref ident, _) = pat.node {
decl.debug_name = ident.node.name;
}
}
decl
}).collect()
});

(
Mir {
basic_blocks: builder.cfg.basic_blocks,
scopes: builder.scope_datas,
var_decls: builder.var_decls,
arg_decls: arg_decls.take().expect("args never built?"),
temp_decls: builder.temp_decls,
upvar_decls: upvar_decls,
return_ty: return_ty,
span: span
},
Expand Down Expand Up @@ -269,7 +295,20 @@ impl<'a,'tcx> Builder<'a,'tcx> {
self.schedule_drop(pattern.as_ref().map_or(ast_block.span, |pat| pat.span),
argument_extent, &lvalue, ty);

ArgDecl { ty: ty, spread: false }
let mut name = token::special_idents::invalid.name;
if let Some(pat) = pattern {
if let hir::PatKind::Ident(_, ref ident, _) = pat.node {
if pat_is_binding(&self.hir.tcx().def_map.borrow(), pat) {
name = ident.node.name;
}
}
}

ArgDecl {
ty: ty,
spread: false,
debug_name: name
}
})
.collect();

Expand Down
31 changes: 19 additions & 12 deletions src/librustc_trans/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,28 @@ fn make_mir_scope(ccx: &CrateContext,
return;
};

scopes[idx] = if !has_variables.contains(idx) {
if !has_variables.contains(idx) {
// Do not create a DIScope if there are no variables
// defined in this MIR Scope, to avoid debuginfo bloat.
parent_scope
} else {
let loc = span_start(ccx, scope_data.span);
let file_metadata = file_metadata(ccx, &loc.file.name);
unsafe {
llvm::LLVMDIBuilderCreateLexicalBlock(
DIB(ccx),
parent_scope,
file_metadata,
loc.line as c_uint,
loc.col.to_usize() as c_uint)

// However, we don't skip creating a nested scope if
// our parent is the root, because we might want to
// put arguments in the root and not have shadowing.
if parent_scope != fn_metadata {
scopes[idx] = parent_scope;
return;
}
}

let loc = span_start(ccx, scope_data.span);
let file_metadata = file_metadata(ccx, &loc.file.name);
scopes[idx] = unsafe {
llvm::LLVMDIBuilderCreateLexicalBlock(
DIB(ccx),
parent_scope,
file_metadata,
loc.line as c_uint,
loc.col.to_usize() as c_uint)
};
}

Expand Down
82 changes: 75 additions & 7 deletions src/librustc_trans/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
let scopes = debuginfo::create_mir_scopes(fcx);

// Allocate variable and temp allocas
let args = arg_value_refs(&bcx, &mir, &scopes);
let vars = mir.var_decls.iter()
.map(|decl| (bcx.monomorphize(&decl.ty), decl))
.map(|(mty, decl)| {
Expand Down Expand Up @@ -156,7 +157,6 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
TempRef::Operand(None)
})
.collect();
let args = arg_value_refs(&bcx, &mir, &scopes);

// Allocate a `Block` for every basic block
let block_bcxs: Vec<Block<'blk,'tcx>> =
Expand Down Expand Up @@ -278,15 +278,15 @@ fn arg_value_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,
let byte_offset_of_var_in_tuple =
machine::llelement_offset(bcx.ccx(), lltuplety, i);

let address_operations = unsafe {
let ops = unsafe {
[llvm::LLVMDIBuilderCreateOpDeref(),
llvm::LLVMDIBuilderCreateOpPlus(),
byte_offset_of_var_in_tuple as i64]
};

let variable_access = VariableAccess::IndirectVariable {
alloca: lltemp,
address_operations: &address_operations
address_operations: &ops
};
declare_local(bcx, token::special_idents::invalid.name,
tupled_arg_ty, scope, variable_access,
Expand Down Expand Up @@ -327,10 +327,78 @@ fn arg_value_refs<'bcx, 'tcx>(bcx: &BlockAndBuilder<'bcx, 'tcx>,
lltemp
};
bcx.with_block(|bcx| arg_scope.map(|scope| {
declare_local(bcx, token::special_idents::invalid.name, arg_ty, scope,
VariableAccess::DirectVariable { alloca: llval },
VariableKind::ArgumentVariable(arg_index + 1),
bcx.fcx().span.unwrap_or(DUMMY_SP));
// Is this a regular argument?
if arg_index > 0 || mir.upvar_decls.is_empty() {
declare_local(bcx, arg_decl.debug_name, arg_ty, scope,
VariableAccess::DirectVariable { alloca: llval },
VariableKind::ArgumentVariable(arg_index + 1),
bcx.fcx().span.unwrap_or(DUMMY_SP));
return;
}

// Or is it the closure environment?
let (closure_ty, env_ref) = if let ty::TyRef(_, mt) = arg_ty.sty {
(mt.ty, true)
} else {
(arg_ty, false)
};
let upvar_tys = if let ty::TyClosure(_, ref substs) = closure_ty.sty {
&substs.upvar_tys[..]
} else {
bug!("upvar_decls with non-closure arg0 type `{}`", closure_ty);
};

// Store the pointer to closure data in an alloca for debuginfo
// because that's what the llvm.dbg.declare intrinsic expects.

// FIXME(eddyb) this shouldn't be necessary but SROA seems to
// mishandle DW_OP_plus not preceded by DW_OP_deref, i.e. it
// doesn't actually strip the offset when splitting the closure
// environment into its components so it ends up out of bounds.
let env_ptr = if !env_ref {
use base::*;
use build::*;
use common::*;
let alloc = alloca(bcx, val_ty(llval), "__debuginfo_env_ptr");
Store(bcx, llval, alloc);
alloc
} else {
llval
};

let llclosurety = type_of::type_of(bcx.ccx(), closure_ty);
for (i, (decl, ty)) in mir.upvar_decls.iter().zip(upvar_tys).enumerate() {
let byte_offset_of_var_in_env =
machine::llelement_offset(bcx.ccx(), llclosurety, i);

let ops = unsafe {
[llvm::LLVMDIBuilderCreateOpDeref(),
llvm::LLVMDIBuilderCreateOpPlus(),
byte_offset_of_var_in_env as i64,
llvm::LLVMDIBuilderCreateOpDeref()]
};

// The environment and the capture can each be indirect.

// FIXME(eddyb) see above why we have to keep
// a pointer in an alloca for debuginfo atm.
let mut ops = if env_ref || true { &ops[..] } else { &ops[1..] };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|| true ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the FIXME there and above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, now I see.


let ty = if let (true, &ty::TyRef(_, mt)) = (decl.by_ref, &ty.sty) {
mt.ty
} else {
ops = &ops[..ops.len() - 1];
ty
};

let variable_access = VariableAccess::IndirectVariable {
alloca: env_ptr,
address_operations: &ops
};
declare_local(bcx, decl.debug_name, ty, scope, variable_access,
VariableKind::CapturedVariable,
bcx.fcx().span.unwrap_or(DUMMY_SP));
}
}));
LvalueRef::new_sized(llval, LvalueTy::from_ty(arg_ty))
}).collect()
Expand Down
3 changes: 1 addition & 2 deletions src/test/debuginfo/associated-types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@

#![allow(unused_variables)]
#![allow(dead_code)]
#![feature(omit_gdb_pretty_printer_section, rustc_attrs)]
#![feature(omit_gdb_pretty_printer_section)]
#![omit_gdb_pretty_printer_section]

trait TraitWithAssocType {
Expand Down Expand Up @@ -127,7 +127,6 @@ fn assoc_tuple<T: TraitWithAssocType>(arg: (T, T::Type)) {
zzz(); // #break
}

#[rustc_no_mir] // FIXME(#32790) MIR reuses scopes for match arms.
fn assoc_enum<T: TraitWithAssocType>(arg: Enum<T>) {

match arg {
Expand Down
Loading