Auto merge of rust-lang#56231 - eddyb:mir-debuginfo, r=oli-obk

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 rust-lang#48300 and further optimizations were blocked on being able to describe complex debuginfo transformations (see rust-lang#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

Author: bors

src/librustc/mir/mod.rs

@@ -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
@@ -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 {
@@ -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,
@@ -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
@@ -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
@@ -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:
@@ -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.
@@ -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,
@@ -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>,
 }
 
 ///////////////////////////////////////////////////////////////////////////
@@ -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)]
@@ -2946,7 +2922,6 @@ CloneTypeFoldableAndLiftImpls! {
     MirPhase,
     Mutability,
     SourceInfo,
-    UpvarDebuginfo,
     FakeReadCause,
     RetagKind,
     SourceScope,

src/librustc/mir/visit.rs

@@ -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,
@@ -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);
             }
 
@@ -687,9 +696,7 @@ macro_rules! make_mir_visitor {
                     mutability: _,
                     ty,
                     user_ty,
-                    name: _,
                     source_info,
-                    visibility_scope,
                     internal: _,
                     local_info: _,
                     is_block_tail: _,
@@ -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,
@@ -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)]

src/librustc_codegen_llvm/debuginfo/create_scope_map.rs

@@ -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.