Skip to content

Commit

Permalink
Auto merge of #56231 - eddyb:mir-debuginfo, r=oli-obk
Browse files Browse the repository at this point in the history 
rustc: move debug info from LocalDecl and UpvarDecl into a dedicated VarDebugInfo.

This PR introduces a MIR "user variable" debuginfo system, which amounts to mapping a variable name, in some `SourceScope`, to a `Place`, so that:

* each name can appear multiple times (e.g. due to macro hygiene), even in the same scope
* each `Place` can appear multiple times (e.g. in the future from optimizations like NRVO, which collapse multiple MIR locals into one)
* the `Place`s aren't limited to just locals, so they can describe the (right now quite ad-hoc) closure upvars and generator saved state fields, and can be properly transformed by optimizations (e.g. inlining - see `src/test/mir-opt/inline-closure-captures.rs`)

The main motivation for this was that #48300 and further optimizations were blocked on being able to describe complex debuginfo transformations (see #48300 (comment)).

<hr/>

In the textual representation, the "user variable" debuginfo can be found in each scope, and consists of `debug NAME => PLACE;` "declarations", e.g. the MIR for `let x = ...; let y = ...; ...` is now:
```rust
    let _1: T;                           // in scope 0 at ...
    scope 1 {
        debug x => _1;                   // in scope 1 at ...
        let _2: T;                       // in scope 1 at ...
        scope 2 {
            debug y => _2;               // in scope 2 at ...
        }
    }
```
For reference, this is how the information was represented before this PR:
(notably, the scopes in which the variables are visible for debuginfo weren't even shown anywhere, making `scope 2` look pointless, and user variable names were part of MIR locals)
```rust
    let _1: T;                           // "x" in scope 0 at ...
    scope 1 {
        let _2: T;                       // "y" in scope 1 at ...
        scope 2 {
        }
    }
```

cc @nikomatsakis @michaelwoerister
  • Loading branch information
@bors
bors committed Nov 27, 2019
2 parents 04e69e4 + 563ed27 commit e87a205
Show file tree
Hide file tree
Showing 49 changed files with 780 additions and 480 deletions.
73 changes: 24 additions & 49 deletions src/librustc/mir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,8 @@ pub struct Body<'tcx> {
/// 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, so please rely on local crate HIR or other side-channels.
pub __upvar_debuginfo_codegen_only_do_not_use: Vec<UpvarDebuginfo>,
/// Debug information pertaining to user variables, including captures.
pub var_debug_info: Vec<VarDebugInfo<'tcx>>,

/// 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
Expand All @@ -170,11 +164,10 @@ impl<'tcx> Body<'tcx> {
basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
source_scopes: IndexVec<SourceScope, SourceScopeData>,
source_scope_local_data: ClearCrossCrate<IndexVec<SourceScope, SourceScopeLocalData>>,
yield_ty: Option<Ty<'tcx>>,
local_decls: LocalDecls<'tcx>,
user_type_annotations: CanonicalUserTypeAnnotations<'tcx>,
arg_count: usize,
__upvar_debuginfo_codegen_only_do_not_use: Vec<UpvarDebuginfo>,
var_debug_info: Vec<VarDebugInfo<'tcx>>,
span: Span,
control_flow_destroyed: Vec<(Span, String)>,
) -> Self {
Expand All @@ -191,14 +184,14 @@ impl<'tcx> Body<'tcx> {
basic_blocks,
source_scopes,
source_scope_local_data,
yield_ty,
yield_ty: None,
generator_drop: None,
generator_layout: None,
local_decls,
user_type_annotations,
arg_count,
__upvar_debuginfo_codegen_only_do_not_use,
spread_arg: None,
var_debug_info,
span,
cache: cache::Cache::new(),
control_flow_destroyed,
Expand Down Expand Up @@ -280,7 +273,7 @@ impl<'tcx> Body<'tcx> {
LocalKind::ReturnPointer
} else if index < self.arg_count + 1 {
LocalKind::Arg
} else if self.local_decls[local].name.is_some() {
} else if self.local_decls[local].is_user_variable() {
LocalKind::Var
} else {
LocalKind::Temp
Expand Down Expand Up @@ -728,12 +721,6 @@ pub struct LocalDecl<'tcx> {
// FIXME(matthewjasper) Don't store in this in `Body`
pub user_ty: UserTypeProjections,

/// The name of the local, used in debuginfo and pretty-printing.
///
/// Note that function arguments can also have this set to `Some(_)`
/// to generate better debuginfo.
pub name: Option<Name>,

/// The *syntactic* (i.e., not visibility) source scope the local is defined
/// in. If the local was defined in a let-statement, this
/// is *within* the let-statement, rather than outside
Expand Down Expand Up @@ -785,9 +772,9 @@ pub struct LocalDecl<'tcx> {
/// `drop(x)`, we want it to refer to `x: u32`.
///
/// To allow both uses to work, we need to have more than a single scope
/// for a local. We have the `source_info.scope` represent the
/// "syntactic" lint scope (with a variable being under its let
/// block) while the `visibility_scope` represents the "local variable"
/// for a local. We have the `source_info.scope` represent the "syntactic"
/// lint scope (with a variable being under its let block) while the
/// `var_debug_info.source_info.scope` represents the "local variable"
/// scope (where the "rest" of a block is under all prior let-statements).
///
/// The end result looks like this:
Expand All @@ -806,18 +793,14 @@ pub struct LocalDecl<'tcx> {
/// │ │
/// │ │ │{ let y: u32 }
/// │ │ │
/// │ │ │← y.visibility_scope
/// │ │ │← y.var_debug_info.source_info.scope
/// │ │ │← `y + 2`
/// │
/// │ │{ let x: u32 }
/// │ │← x.visibility_scope
/// │ │← x.var_debug_info.source_info.scope
/// │ │← `drop(x)` // This accesses `x: u32`.
/// ```
pub source_info: SourceInfo,

/// Source scope within which the local is visible (for debuginfo)
/// (see `source_info` for more details).
pub visibility_scope: SourceScope,
}

/// Extra information about a local that's used for diagnostics.
Expand Down Expand Up @@ -955,9 +938,7 @@ impl<'tcx> LocalDecl<'tcx> {
mutability,
ty,
user_ty: UserTypeProjections::none(),
name: None,
source_info: SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE },
visibility_scope: OUTERMOST_SOURCE_SCOPE,
internal,
local_info: LocalInfo::Other,
is_block_tail: None,
Expand All @@ -974,22 +955,27 @@ impl<'tcx> LocalDecl<'tcx> {
ty: return_ty,
user_ty: UserTypeProjections::none(),
source_info: SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE },
visibility_scope: OUTERMOST_SOURCE_SCOPE,
internal: false,
is_block_tail: None,
name: None, // FIXME maybe we do want some name here?
local_info: LocalInfo::Other,
}
}
}

/// A closure capture, with its name and mode.
#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
pub struct UpvarDebuginfo {
pub debug_name: Name,
/// Debug information pertaining to a user variable.
#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
pub struct VarDebugInfo<'tcx> {
pub name: Name,

/// If true, the capture is behind a reference.
pub by_ref: bool,
/// Source info of the user variable, including the scope
/// within which the variable is visible (to debuginfo)
/// (see `LocalDecl`'s `source_info` field for more details).
pub source_info: SourceInfo,

/// Where the data for this user variable is to be found.
/// NOTE(eddyb) There's an unenforced invariant that this `Place` is
/// based on a `Local`, not a `Static`, and contains no indexing.
pub place: Place<'tcx>,
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2758,16 +2744,6 @@ pub struct GeneratorLayout<'tcx> {
/// have conflicts with each other are allowed to overlap in the computed
/// layout.
pub storage_conflicts: BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal>,

/// The names and scopes of all the stored generator locals.
///
/// N.B., this is *strictly* a temporary hack for codegen
/// debuginfo generation, and will be removed at some point.
/// Do **NOT** use it for anything else, local information should not be
/// in the MIR, please rely on local crate HIR or other side-channels.
//
// FIXME(tmandry): see above.
pub __local_debuginfo_codegen_only_do_not_use: IndexVec<GeneratorSavedLocal, LocalDecl<'tcx>>,
}

#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
Expand Down Expand Up @@ -2946,7 +2922,6 @@ CloneTypeFoldableAndLiftImpls! {
MirPhase,
Mutability,
SourceInfo,
UpvarDebuginfo,
FakeReadCause,
RetagKind,
SourceScope,
Expand Down
31 changes: 28 additions & 3 deletions src/librustc/mir/visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ macro_rules! make_mir_visitor {
self.super_local_decl(local, local_decl);
}

fn visit_var_debug_info(&mut self,
var_debug_info: & $($mutability)* VarDebugInfo<'tcx>) {
self.super_var_debug_info(var_debug_info);
}

fn visit_local(&mut self,
_local: & $($mutability)? Local,
_context: PlaceContext,
Expand Down Expand Up @@ -279,6 +284,10 @@ macro_rules! make_mir_visitor {
);
}

for var_debug_info in &$($mutability)? body.var_debug_info {
self.visit_var_debug_info(var_debug_info);
}

self.visit_span(&$($mutability)? body.span);
}

Expand Down Expand Up @@ -687,9 +696,7 @@ macro_rules! make_mir_visitor {
mutability: _,
ty,
user_ty,
name: _,
source_info,
visibility_scope,
internal: _,
local_info: _,
is_block_tail: _,
Expand All @@ -703,7 +710,23 @@ macro_rules! make_mir_visitor {
self.visit_user_type_projection(user_ty);
}
self.visit_source_info(source_info);
self.visit_source_scope(visibility_scope);
}

fn super_var_debug_info(&mut self,
var_debug_info: & $($mutability)? VarDebugInfo<'tcx>) {
let VarDebugInfo {
name: _,
source_info,
place,
} = var_debug_info;

self.visit_source_info(source_info);
let location = START_BLOCK.start_location();
self.visit_place(
place,
PlaceContext::NonUse(NonUseContext::VarDebugInfo),
location,
);
}

fn super_source_scope(&mut self,
Expand Down Expand Up @@ -1029,6 +1052,8 @@ pub enum NonUseContext {
StorageDead,
/// User type annotation assertions for NLL.
AscribeUserTy,
/// The data of an user variable, for debug info.
VarDebugInfo,
}

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_codegen_llvm/debuginfo/create_scope_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ pub fn compute_mir_scopes(
) {
// Find all the scopes with variables defined in them.
let mut has_variables = BitSet::new_empty(mir.source_scopes.len());
// FIXME(eddyb) base this on `decl.name`, or even better, on debuginfo.
// FIXME(eddyb) take into account that arguments always have debuginfo,
// irrespective of their name (assuming full debuginfo is enabled).
for var in mir.vars_iter() {
let decl = &mir.local_decls[var];
has_variables.insert(decl.visibility_scope);
for var_debug_info in &mir.var_debug_info {
has_variables.insert(var_debug_info.source_info.scope);
}

// Instantiate all scopes.
Expand Down
Loading

0 comments on commit e87a205

Please sign in to comment.