Auto merge of rust-lang#115252 - cjgillot:mir-composite, r=davidtwco

Represent MIR composite debuginfo as projections instead of aggregates

Composite debuginfo for MIR is currently represented as
```
debug name => Type { projection1 => place1, projection2 => place2 };
```
ie. a single `VarDebugInfo` object with that name, and its value a `VarDebugInfoContents::Composite`.

This PR proposes to reverse the representation to be
```
debug name.projection1 => place1;
debug name.projection2 => place2;
```
ie. multiple `VarDebugInfo` objects with each their projection.

This simplifies the handling of composite debuginfo by the compiler by avoiding weird nesting.

Based on rust-lang#115139

Author: bors

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

@@ -484,54 +484,89 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                 None
             };
 
-            let dbg_var = dbg_scope_and_span.map(|(dbg_scope, _, span)| {
-                let (var_ty, var_kind) = match var.value {
+            let var_ty = if let Some(ref fragment) = var.composite {
+                self.monomorphize(fragment.ty)
+            } else {
+                match var.value {
                     mir::VarDebugInfoContents::Place(place) => {
-                        let var_ty = self.monomorphized_place_ty(place.as_ref());
-                        let var_kind = if let Some(arg_index) = var.argument_index
-                            && place.projection.is_empty()
-                        {
-                            let arg_index = arg_index as usize;
-                            if target_is_msvc {
-                                // ScalarPair parameters are spilled to the stack so they need to
-                                // be marked as a `LocalVariable` for MSVC debuggers to visualize
-                                // their data correctly. (See #81894 & #88625)
-                                let var_ty_layout = self.cx.layout_of(var_ty);
-                                if let Abi::ScalarPair(_, _) = var_ty_layout.abi {
-                                    VariableKind::LocalVariable
-                                } else {
-                                    VariableKind::ArgumentVariable(arg_index)
-                                }
-                            } else {
-                                // FIXME(eddyb) shouldn't `ArgumentVariable` indices be
-                                // offset in closures to account for the hidden environment?
-                                VariableKind::ArgumentVariable(arg_index)
-                            }
-                        } else {
-                            VariableKind::LocalVariable
-                        };
-                        (var_ty, var_kind)
+                        self.monomorphized_place_ty(place.as_ref())
                     }
-                    mir::VarDebugInfoContents::Const(c) => {
-                        let ty = self.monomorphize(c.ty());
-                        (ty, VariableKind::LocalVariable)
-                    }
-                    mir::VarDebugInfoContents::Composite { ty, fragments: _ } => {
-                        let ty = self.monomorphize(ty);
-                        (ty, VariableKind::LocalVariable)
+                    mir::VarDebugInfoContents::Const(c) => self.monomorphize(c.ty()),
+                }
+            };
+
+            let dbg_var = dbg_scope_and_span.map(|(dbg_scope, _, span)| {
+                let var_kind = if let Some(arg_index) = var.argument_index
+                    && var.composite.is_none()
+                    && let mir::VarDebugInfoContents::Place(place) = var.value
+                    && place.projection.is_empty()
+                {
+                    let arg_index = arg_index as usize;
+                    if target_is_msvc {
+                        // ScalarPair parameters are spilled to the stack so they need to
+                        // be marked as a `LocalVariable` for MSVC debuggers to visualize
+                        // their data correctly. (See #81894 & #88625)
+                        let var_ty_layout = self.cx.layout_of(var_ty);
+                        if let Abi::ScalarPair(_, _) = var_ty_layout.abi {
+                            VariableKind::LocalVariable
+                        } else {
+                            VariableKind::ArgumentVariable(arg_index)
+                        }
+                    } else {
+                        // FIXME(eddyb) shouldn't `ArgumentVariable` indices be
+                        // offset in closures to account for the hidden environment?
+                        VariableKind::ArgumentVariable(arg_index)
                     }
+                } else {
+                    VariableKind::LocalVariable
                 };
 
                 self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span)
             });
 
+            let fragment = if let Some(ref fragment) = var.composite {
+                let var_layout = self.cx.layout_of(var_ty);
+
+                let mut fragment_start = Size::ZERO;
+                let mut fragment_layout = var_layout;
+
+                for elem in &fragment.projection {
+                    match *elem {
+                        mir::ProjectionElem::Field(field, _) => {
+                            let i = field.index();
+                            fragment_start += fragment_layout.fields.offset(i);
+                            fragment_layout = fragment_layout.field(self.cx, i);
+                        }
+                        _ => span_bug!(
+                            var.source_info.span,
+                            "unsupported fragment projection `{:?}`",
+                            elem,
+                        ),
+                    }
+                }
+
+                if fragment_layout.size == Size::ZERO {
+                    // Fragment is a ZST, so does not represent anything. Avoid generating anything
+                    // as this may conflict with a fragment that covers the entire variable.
+                    continue;
+                } else if fragment_layout.size == var_layout.size {
+                    // Fragment covers entire variable, so as far as
+                    // DWARF is concerned, it's not really a fragment.
+                    None
+                } else {
+                    Some(fragment_start..fragment_start + fragment_layout.size)
+                }
+            } else {
+                None
+            };
+
             match var.value {
                 mir::VarDebugInfoContents::Place(place) => {
                     per_local[place.local].push(PerLocalVarDebugInfo {
                         name: var.name,
                         source_info: var.source_info,
                         dbg_var,
-                        fragment: None,
+                        fragment,
                         projection: place.projection,
                     });
                 }
@@ -547,51 +582,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                                 bx,
                             );
 
-                            bx.dbg_var_addr(dbg_var, dbg_loc, base.llval, Size::ZERO, &[], None);
-                        }
-                    }
-                }
-                mir::VarDebugInfoContents::Composite { ty, ref fragments } => {
-                    let var_ty = self.monomorphize(ty);
-                    let var_layout = self.cx.layout_of(var_ty);
-                    for fragment in fragments {
-                        let mut fragment_start = Size::ZERO;
-                        let mut fragment_layout = var_layout;
-
-                        for elem in &fragment.projection {
-                            match *elem {
-                                mir::ProjectionElem::Field(field, _) => {
-                                    let i = field.index();
-                                    fragment_start += fragment_layout.fields.offset(i);
-                                    fragment_layout = fragment_layout.field(self.cx, i);
-                                }
-                                _ => span_bug!(
-                                    var.source_info.span,
-                                    "unsupported fragment projection `{:?}`",
-                                    elem,
-                                ),
-                            }
+                            bx.dbg_var_addr(
+                                dbg_var,
+                                dbg_loc,
+                                base.llval,
+                                Size::ZERO,
+                                &[],
+                                fragment,
+                            );
                         }
-
-                        let place = fragment.contents;
-                        let fragment = if fragment_layout.size == Size::ZERO {
-                            // Fragment is a ZST, so does not represent anything.
-                            continue;
-                        } else if fragment_layout.size == var_layout.size {
-                            // Fragment covers entire variable, so as far as
-                            // DWARF is concerned, it's not really a fragment.
-                            None
-                        } else {
-                            Some(fragment_start..fragment_start + fragment_layout.size)
-                        };
-
-                        per_local[place.local].push(PerLocalVarDebugInfo {
-                            name: var.name,
-                            source_info: var.source_info,
-                            dbg_var,
-                            fragment,
-                            projection: place.projection,
-                        });
                     }
                 }
             }

compiler/rustc_const_eval/src/transform/validate.rs

@@ -6,13 +6,7 @@ use rustc_index::IndexVec;
 use rustc_infer::traits::Reveal;
 use rustc_middle::mir::interpret::Scalar;
 use rustc_middle::mir::visit::{NonUseContext, PlaceContext, Visitor};
-use rustc_middle::mir::{
-    traversal, BasicBlock, BinOp, Body, BorrowKind, CastKind, CopyNonOverlapping, Local, Location,
-    MirPass, MirPhase, NonDivergingIntrinsic, NullOp, Operand, Place, PlaceElem, PlaceRef,
-    ProjectionElem, RetagKind, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind,
-    Terminator, TerminatorKind, UnOp, UnwindAction, UnwindTerminateReason, VarDebugInfo,
-    VarDebugInfoContents, START_BLOCK,
-};
+use rustc_middle::mir::*;
 use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitableExt};
 use rustc_mir_dataflow::impls::MaybeStorageLive;
 use rustc_mir_dataflow::storage::always_storage_live_locals;
@@ -757,37 +751,37 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
     }
 
     fn visit_var_debug_info(&mut self, debuginfo: &VarDebugInfo<'tcx>) {
-        let check_place = |this: &mut Self, place: Place<'_>| {
-            if place.projection.iter().any(|p| !p.can_use_in_debuginfo()) {
-                this.fail(
+        if let Some(box VarDebugInfoFragment { ty, ref projection }) = debuginfo.composite {
+            if ty.is_union() || ty.is_enum() {
+                self.fail(
                     START_BLOCK.start_location(),
-                    format!("illegal place {:?} in debuginfo for {:?}", place, debuginfo.name),
+                    format!("invalid type {ty:?} in debuginfo for {:?}", debuginfo.name),
                 );
             }
-        };
+            if projection.is_empty() {
+                self.fail(
+                    START_BLOCK.start_location(),
+                    format!("invalid empty projection in debuginfo for {:?}", debuginfo.name),
+                );
+            }
+            if projection.iter().any(|p| !matches!(p, PlaceElem::Field(..))) {
+                self.fail(
+                    START_BLOCK.start_location(),
+                    format!(
+                        "illegal projection {:?} in debuginfo for {:?}",
+                        projection, debuginfo.name
+                    ),
+                );
+            }
+        }
         match debuginfo.value {
             VarDebugInfoContents::Const(_) => {}
             VarDebugInfoContents::Place(place) => {
-                check_place(self, place);
-            }
-            VarDebugInfoContents::Composite { ty, ref fragments } => {
-                for f in fragments {
-                    check_place(self, f.contents);
-                    if ty.is_union() || ty.is_enum() {
-                        self.fail(
-                            START_BLOCK.start_location(),
-                            format!("invalid type {ty:?} for composite debuginfo"),
-                        );
-                    }
-                    if f.projection.iter().any(|p| !matches!(p, PlaceElem::Field(..))) {
-                        self.fail(
-                            START_BLOCK.start_location(),
-                            format!(
-                                "illegal projection {:?} in debuginfo for {:?}",
-                                f.projection, debuginfo.name
-                            ),
-                        );
-                    }
+                if place.projection.iter().any(|p| !p.can_use_in_debuginfo()) {
+                    self.fail(
+                        START_BLOCK.start_location(),
+                        format!("illegal place {:?} in debuginfo for {:?}", place, debuginfo.name),
+                    );
                 }
             }
         }