Skip to content
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

Future-proof MIR for dedicated debuginfo. #56278

Merged
merged 9 commits into from
Apr 24, 2019
31 changes: 15 additions & 16 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::hir::def::{CtorKind, Namespace};
use crate::hir::def_id::DefId;
use crate::hir::{self, HirId, InlineAsm as HirInlineAsm};
use crate::hir::{self, InlineAsm as HirInlineAsm};
use crate::mir::interpret::{ConstValue, InterpError, Scalar};
use crate::mir::visit::MirVisitable;
use rustc_apfloat::ieee::{Double, Single};
Expand Down Expand Up @@ -138,16 +138,20 @@ pub struct Mir<'tcx> {
/// If this MIR was built for a constant, this will be 0.
pub arg_count: usize,

/// 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>,

/// Mark an argument local (which must be a tuple) as getting passed as
/// its individual components at the LLVM level.
///
/// This is used for the "rust-call" ABI.
pub spread_arg: Option<Local>,

/// Names and capture modes of all the closure upvars, assuming
/// the first argument is either the closure or a reference to it.
// NOTE(eddyb) This is *strictly* a temporary hack for codegen
// debuginfo generation, and will be removed at some point.
// Do **NOT** use it for anything else, upvar information should not be
// in the MIR, please rely on local crate HIR or other side-channels.
pub __upvar_debuginfo_codegen_only_do_not_use: Vec<UpvarDebuginfo>,
Copy link
Contributor

Choose a reason for hiding this comment

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

lol ok I guess that addresses my earlier concern

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like this to be part of the doc comment please though, why use only //?

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 got used to only using // for NOTE/FIXME/HACK, but I'll go change it.


/// Mark this MIR of a const context other than const functions as having converted a `&&` or
/// `||` expression into `&` or `|` respectively. This is problematic because if we ever stop
/// this conversion from happening and use short circuiting, we will cause the following code
Expand All @@ -173,7 +177,7 @@ impl<'tcx> Mir<'tcx> {
local_decls: LocalDecls<'tcx>,
user_type_annotations: CanonicalUserTypeAnnotations<'tcx>,
arg_count: usize,
upvar_decls: Vec<UpvarDecl>,
__upvar_debuginfo_codegen_only_do_not_use: Vec<UpvarDebuginfo>,
span: Span,
control_flow_destroyed: Vec<(Span, String)>,
) -> Self {
Expand All @@ -197,7 +201,7 @@ impl<'tcx> Mir<'tcx> {
local_decls,
user_type_annotations,
arg_count,
upvar_decls,
__upvar_debuginfo_codegen_only_do_not_use,
spread_arg: None,
span,
cache: cache::Cache::new(),
Expand Down Expand Up @@ -431,7 +435,7 @@ impl_stable_hash_for!(struct Mir<'tcx> {
local_decls,
user_type_annotations,
arg_count,
upvar_decls,
__upvar_debuginfo_codegen_only_do_not_use,
spread_arg,
control_flow_destroyed,
span,
Expand Down Expand Up @@ -983,16 +987,11 @@ impl<'tcx> LocalDecl<'tcx> {

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

/// `HirId` of the captured variable
pub var_hir_id: ClearCrossCrate<HirId>,

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

pub mutability: Mutability,
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -3156,7 +3155,7 @@ CloneTypeFoldableAndLiftImpls! {
MirPhase,
Mutability,
SourceInfo,
UpvarDecl,
UpvarDebuginfo,
FakeReadCause,
RetagKind,
SourceScope,
Expand All @@ -3178,7 +3177,7 @@ BraceStructTypeFoldableImpl! {
local_decls,
user_type_annotations,
arg_count,
upvar_decls,
__upvar_debuginfo_codegen_only_do_not_use,
spread_arg,
control_flow_destroyed,
span,
Expand Down
35 changes: 0 additions & 35 deletions src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,41 +133,6 @@ impl<'tcx> Place<'tcx> {
proj.base.ty(local_decls, tcx).projection_ty(tcx, &proj.elem),
}
}

/// If this is a field projection, and the field is being projected from a closure type,
/// then returns the index of the field being projected. Note that this closure will always
/// be `self` in the current MIR, because that is the only time we directly access the fields
/// of a closure type.
pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>,
tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option<Field> {
let (place, by_ref) = if let Place::Projection(ref proj) = self {
if let ProjectionElem::Deref = proj.elem {
(&proj.base, true)
} else {
(self, false)
}
} else {
(self, false)
};

match place {
Place::Projection(ref proj) => match proj.elem {
ProjectionElem::Field(field, _ty) => {
let base_ty = proj.base.ty(mir, *tcx).ty;

if (base_ty.is_closure() || base_ty.is_generator()) &&
(!by_ref || mir.upvar_decls[field.index()].by_ref)
{
Some(field)
} else {
None
}
},
_ => None,
}
_ => None,
}
}
}

pub enum RvalueInitializationState {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
finalize(self)
}

fn debuginfo_upvar_decls_ops_sequence(&self, byte_offset_of_var_in_env: u64) -> [i64; 4] {
fn debuginfo_upvar_ops_sequence(&self, byte_offset_of_var_in_env: u64) -> [i64; 4] {
unsafe {
[llvm::LLVMRustDIBuilderCreateOpDeref(),
llvm::LLVMRustDIBuilderCreateOpPlusUconst(),
Expand Down
13 changes: 7 additions & 6 deletions src/librustc_codegen_ssa/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,9 +598,10 @@ fn arg_local_refs<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
tmp
}
};
let upvar_debuginfo = &mir.__upvar_debuginfo_codegen_only_do_not_use;
arg_scope.map(|scope| {
// Is this a regular argument?
if arg_index > 0 || mir.upvar_decls.is_empty() {
if arg_index > 0 || upvar_debuginfo.is_empty() {
// The Rust ABI passes indirect variables using a pointer and a manual copy, so we
// need to insert a deref here, but the C ABI uses a pointer and a copy using the
// byval attribute, for which LLVM always does the deref itself,
Expand Down Expand Up @@ -638,16 +639,16 @@ fn arg_local_refs<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
let (def_id, upvar_substs) = match closure_layout.ty.sty {
ty::Closure(def_id, substs) => (def_id, UpvarSubsts::Closure(substs)),
ty::Generator(def_id, substs, _) => (def_id, UpvarSubsts::Generator(substs)),
_ => bug!("upvar_decls with non-closure arg0 type `{}`", closure_layout.ty)
_ => bug!("upvar debuginfo with non-closure arg0 type `{}`", closure_layout.ty)
};
let upvar_tys = upvar_substs.upvar_tys(def_id, tcx);

let extra_locals = {
let upvars = mir.upvar_decls
let upvars = upvar_debuginfo
.iter()
.zip(upvar_tys)
.enumerate()
.map(|(i, (decl, ty))| (i, decl.debug_name, decl.by_ref, ty));
.map(|(i, (upvar, ty))| (i, upvar.debug_name, upvar.by_ref, ty));

let generator_fields = mir.generator_layout.as_ref().map(|generator_layout| {
let (def_id, gen_substs) = match closure_layout.ty.sty {
Expand All @@ -656,7 +657,7 @@ fn arg_local_refs<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
};
let state_tys = gen_substs.state_tys(def_id, tcx);

let upvar_count = mir.upvar_decls.len();
let upvar_count = upvar_debuginfo.len();
generator_layout.fields
.iter()
.zip(state_tys)
Expand All @@ -673,7 +674,7 @@ fn arg_local_refs<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
for (field, name, by_ref, ty) in extra_locals {
let byte_offset_of_var_in_env = closure_layout.fields.offset(field).bytes();

let ops = bx.debuginfo_upvar_decls_ops_sequence(byte_offset_of_var_in_env);
let ops = bx.debuginfo_upvar_ops_sequence(byte_offset_of_var_in_env);

// The environment and the capture can each be indirect.
let mut ops = if env_ref { &ops[..] } else { &ops[1..] };
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/traits/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub trait DebugInfoMethods<'tcx>: BackendTypes {
defining_crate: CrateNum,
) -> Self::DIScope;
fn debuginfo_finalize(&self);
fn debuginfo_upvar_decls_ops_sequence(&self, byte_offset_of_var_in_env: u64) -> [i64; 4];
fn debuginfo_upvar_ops_sequence(&self, byte_offset_of_var_in_env: u64) -> [i64; 4];
}

pub trait DebugInfoBuilderMethods<'tcx>: BackendTypes {
Expand Down
12 changes: 6 additions & 6 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
| LocalKind::Temp => bug!("temporary or return pointer with a name"),
LocalKind::Var => "local variable ",
LocalKind::Arg
if !self.mir.upvar_decls.is_empty()
if !self.upvars.is_empty()
&& local == Local::new(1) => {
"variable captured by `move` "
}
Expand Down Expand Up @@ -1632,11 +1632,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
match proj.elem {
ProjectionElem::Deref => {
let upvar_field_projection =
place.is_upvar_field_projection(self.mir, &self.infcx.tcx);
self.is_upvar_field_projection(place);
if let Some(field) = upvar_field_projection {
let var_index = field.index();
let name = self.mir.upvar_decls[var_index].debug_name.to_string();
if self.mir.upvar_decls[var_index].by_ref {
let name = self.upvars[var_index].name.to_string();
if self.upvars[var_index].by_ref {
buf.push_str(&name);
} else {
buf.push_str(&format!("*{}", &name));
Expand Down Expand Up @@ -1694,10 +1694,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
autoderef = true;

let upvar_field_projection =
place.is_upvar_field_projection(self.mir, &self.infcx.tcx);
self.is_upvar_field_projection(place);
if let Some(field) = upvar_field_projection {
let var_index = field.index();
let name = self.mir.upvar_decls[var_index].debug_name.to_string();
let name = self.upvars[var_index].name.to_string();
buf.push_str(&name);
} else {
let field_name = self.describe_field(&proj.base, field);
Expand Down
Loading