Rollup merge of #68960 - eddyb:llvm-dbg-cleanup, r=nagisa

codegen: misc cleanups around debuginfo scopes and locations.

See each commit message. Most of these seem to be leftovers from the transition to MIR codegen.

r? @nagisa cc @bjorn3

Author: Dylan-DPC

src/librustc_codegen_llvm/debuginfo/create_scope_map.rs

@@ -66,14 +66,8 @@ fn make_mir_scope(
     if !has_variables.contains(scope) {
         // Do not create a DIScope if there are no variables
         // defined in this MIR Scope, to avoid debuginfo bloat.
-
-        // 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.scope_metadata.unwrap() != fn_metadata {
-            debug_context.scopes[scope] = parent_scope;
-            return;
-        }
+        debug_context.scopes[scope] = parent_scope;
+        return;
     }
 
     let loc = span_start(cx, scope_data.span);

src/librustc_codegen_llvm/debuginfo/mod.rs

@@ -5,7 +5,6 @@ use rustc_codegen_ssa::mir::debuginfo::VariableKind::*;
 
 use self::metadata::{file_metadata, type_metadata, TypeMap};
 use self::namespace::mangled_name_of_instance;
-use self::source_loc::InternalDebugLocation::{self, UnknownLocation};
 use self::type_names::compute_debuginfo_type_name;
 use self::utils::{create_DIArray, is_node_local_to_unit, span_start, DIB};
 
@@ -38,7 +37,7 @@ use std::ffi::CString;
 use rustc::ty::layout::{self, HasTyCtxt, LayoutOf, Size};
 use rustc_codegen_ssa::traits::*;
 use rustc_span::symbol::Symbol;
-use rustc_span::{self, BytePos, Pos, Span};
+use rustc_span::{self, BytePos, Span};
 use smallvec::SmallVec;
 use syntax::ast;
 
@@ -52,7 +51,6 @@ mod utils;
 pub use self::create_scope_map::compute_mir_scopes;
 pub use self::metadata::create_global_var_metadata;
 pub use self::metadata::extend_scope_to_file;
-pub use self::source_loc::set_source_location;
 
 #[allow(non_upper_case_globals)]
 const DW_TAG_auto_variable: c_uint = 0x100;
@@ -148,20 +146,18 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> {
     // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.).
     fn dbg_var_addr(
         &mut self,
-        dbg_context: &FunctionDebugContext<&'ll DIScope>,
         dbg_var: &'ll DIVariable,
         scope_metadata: &'ll DIScope,
         variable_alloca: Self::Value,
         direct_offset: Size,
         indirect_offsets: &[Size],
         span: Span,
     ) {
-        assert!(!dbg_context.source_locations_enabled);
         let cx = self.cx();
 
-        let loc = span_start(cx, span);
-
         // Convert the direct and indirect offsets to address ops.
+        // FIXME(eddyb) use `const`s instead of getting the values via FFI,
+        // the values should match the ones in the DWARF standard anyway.
         let op_deref = || unsafe { llvm::LLVMRustDIBuilderCreateOpDeref() };
         let op_plus_uconst = || unsafe { llvm::LLVMRustDIBuilderCreateOpPlusUconst() };
         let mut addr_ops = SmallVec::<[_; 8]>::new();
@@ -178,37 +174,32 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> {
             }
         }
 
-        // FIXME(eddyb) maybe this information could be extracted from `var`,
+        // FIXME(eddyb) maybe this information could be extracted from `dbg_var`,
         // to avoid having to pass it down in both places?
-        source_loc::set_debug_location(
-            self,
-            InternalDebugLocation::new(scope_metadata, loc.line, loc.col.to_usize()),
-        );
+        // NB: `var` doesn't seem to know about the column, so that's a limitation.
+        let dbg_loc = cx.create_debug_loc(scope_metadata, span);
         unsafe {
-            let debug_loc = llvm::LLVMGetCurrentDebugLocation(self.llbuilder);
             // FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`.
-            let instr = llvm::LLVMRustDIBuilderInsertDeclareAtEnd(
+            llvm::LLVMRustDIBuilderInsertDeclareAtEnd(
                 DIB(cx),
                 variable_alloca,
                 dbg_var,
                 addr_ops.as_ptr(),
                 addr_ops.len() as c_uint,
-                debug_loc,
+                dbg_loc,
                 self.llbb(),
             );
-
-            llvm::LLVMSetInstDebugLocation(self.llbuilder, instr);
         }
-        source_loc::set_debug_location(self, UnknownLocation);
     }
 
-    fn set_source_location(
-        &mut self,
-        debug_context: &mut FunctionDebugContext<&'ll DIScope>,
-        scope: &'ll DIScope,
-        span: Span,
-    ) {
-        set_source_location(debug_context, &self, scope, span)
+    fn set_source_location(&mut self, scope: &'ll DIScope, span: Span) {
+        debug!("set_source_location: {}", self.sess().source_map().span_to_string(span));
+
+        let dbg_loc = self.cx().create_debug_loc(scope, span);
+
+        unsafe {
+            llvm::LLVMSetCurrentDebugLocation(self.llbuilder, dbg_loc);
+        }
     }
     fn insert_reference_to_gdb_debug_scripts_section_global(&mut self) {
         gdb::insert_reference_to_gdb_debug_scripts_section_global(self)
@@ -342,7 +333,6 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         };
         let mut fn_debug_context = FunctionDebugContext {
             scopes: IndexVec::from_elem(null_scope, &mir.source_scopes),
-            source_locations_enabled: false,
             defining_crate: def_id.krate,
         };
 

src/librustc_codegen_llvm/debuginfo/source_loc.rs

@@ -1,79 +1,35 @@
-use self::InternalDebugLocation::*;
-
 use super::metadata::UNKNOWN_COLUMN_NUMBER;
 use super::utils::{debug_context, span_start};
-use rustc_codegen_ssa::mir::debuginfo::FunctionDebugContext;
 
-use crate::builder::Builder;
-use crate::llvm;
+use crate::common::CodegenCx;
 use crate::llvm::debuginfo::DIScope;
-use log::debug;
+use crate::llvm::{self, Value};
 use rustc_codegen_ssa::traits::*;
 
 use libc::c_uint;
 use rustc_span::{Pos, Span};
 
-/// Sets the current debug location at the beginning of the span.
-///
-/// Maps to a call to llvm::LLVMSetCurrentDebugLocation(...).
-pub fn set_source_location<D>(
-    debug_context: &FunctionDebugContext<D>,
-    bx: &Builder<'_, 'll, '_>,
-    scope: &'ll DIScope,
-    span: Span,
-) {
-    let dbg_loc = if debug_context.source_locations_enabled {
-        debug!("set_source_location: {}", bx.sess().source_map().span_to_string(span));
-        let loc = span_start(bx.cx(), span);
-        InternalDebugLocation::new(scope, loc.line, loc.col.to_usize())
-    } else {
-        UnknownLocation
-    };
-    set_debug_location(bx, dbg_loc);
-}
-
-#[derive(Copy, Clone, PartialEq)]
-pub enum InternalDebugLocation<'ll> {
-    KnownLocation { scope: &'ll DIScope, line: usize, col: usize },
-    UnknownLocation,
-}
-
-impl InternalDebugLocation<'ll> {
-    pub fn new(scope: &'ll DIScope, line: usize, col: usize) -> Self {
-        KnownLocation { scope, line, col }
-    }
-}
-
-pub fn set_debug_location(bx: &Builder<'_, 'll, '_>, debug_location: InternalDebugLocation<'ll>) {
-    let metadata_node = match debug_location {
-        KnownLocation { scope, line, col } => {
-            // For MSVC, set the column number to zero.
-            // Otherwise, emit it. This mimics clang behaviour.
-            // See discussion in https://github.com/rust-lang/rust/issues/42921
-            let col_used = if bx.sess().target.target.options.is_like_msvc {
-                UNKNOWN_COLUMN_NUMBER
-            } else {
-                col as c_uint
-            };
-            debug!("setting debug location to {} {}", line, col);
-
-            unsafe {
-                Some(llvm::LLVMRustDIBuilderCreateDebugLocation(
-                    debug_context(bx.cx()).llcontext,
-                    line as c_uint,
-                    col_used,
-                    scope,
-                    None,
-                ))
-            }
+impl CodegenCx<'ll, '_> {
+    pub fn create_debug_loc(&self, scope: &'ll DIScope, span: Span) -> &'ll Value {
+        let loc = span_start(self, span);
+
+        // For MSVC, set the column number to zero.
+        // Otherwise, emit it. This mimics clang behaviour.
+        // See discussion in https://github.com/rust-lang/rust/issues/42921
+        let col_used = if self.sess().target.target.options.is_like_msvc {
+            UNKNOWN_COLUMN_NUMBER
+        } else {
+            loc.col.to_usize() as c_uint
+        };
+
+        unsafe {
+            llvm::LLVMRustDIBuilderCreateDebugLocation(
+                debug_context(self).llcontext,
+                loc.line as c_uint,
+                col_used,
+                scope,
+                None,
+            )
         }
-        UnknownLocation => {
-            debug!("clearing debug location ");
-            None
-        }
-    };
-
-    unsafe {
-        llvm::LLVMSetCurrentDebugLocation(bx.llbuilder, metadata_node);
     }
 }

src/librustc_codegen_llvm/llvm/ffi.rs

@@ -909,9 +909,7 @@ extern "C" {
     pub fn LLVMDisposeBuilder(Builder: &'a mut Builder<'a>);
 
     // Metadata
-    pub fn LLVMSetCurrentDebugLocation(Builder: &Builder<'a>, L: Option<&'a Value>);
-    pub fn LLVMGetCurrentDebugLocation(Builder: &Builder<'a>) -> &'a Value;
-    pub fn LLVMSetInstDebugLocation(Builder: &Builder<'a>, Inst: &'a Value);
+    pub fn LLVMSetCurrentDebugLocation(Builder: &Builder<'a>, L: &'a Value);
 
     // Terminators
     pub fn LLVMBuildRetVoid(B: &Builder<'a>) -> &'a Value;

src/librustc_codegen_ssa/mir/debuginfo.rs

@@ -14,7 +14,6 @@ use super::{FunctionCx, LocalRef};
 
 pub struct FunctionDebugContext<D> {
     pub scopes: IndexVec<mir::SourceScope, DebugScope<D>>,
-    pub source_locations_enabled: bool,
     pub defining_crate: CrateNum,
 }
 
@@ -53,11 +52,10 @@ impl<D> DebugScope<D> {
 }
 
 impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
-    pub fn set_debug_loc(&mut self, bx: &mut Bx, source_info: mir::SourceInfo) {
+    pub fn set_debug_loc(&self, bx: &mut Bx, source_info: mir::SourceInfo) {
         let (scope, span) = self.debug_loc(source_info);
-        if let Some(debug_context) = &mut self.debug_context {
-            // FIXME(eddyb) get rid of this unwrap somehow.
-            bx.set_source_location(debug_context, scope.unwrap(), span);
+        if let Some(scope) = scope {
+            bx.set_source_location(scope, span);
         }
     }
 
@@ -210,11 +208,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             return;
         }
 
-        let debug_context = match &self.debug_context {
-            Some(debug_context) => debug_context,
-            None => return,
-        };
-
         // FIXME(eddyb) add debuginfo for unsized places too.
         let base = match local_ref {
             LocalRef::Place(place) => place,
@@ -264,7 +257,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             if let Some(scope) = scope {
                 if let Some(dbg_var) = var.dbg_var {
                     bx.dbg_var_addr(
-                        debug_context,
                         dbg_var,
                         scope,
                         base.llval,

src/librustc_codegen_ssa/mir/mod.rs

@@ -230,13 +230,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         bx.br(fx.blocks[mir::START_BLOCK]);
     }
 
-    // Up until here, IR instructions for this function have explicitly not been annotated with
-    // source code location, so we don't step into call setup code. From here on, source location
-    // emitting should be enabled.
-    if let Some(debug_context) = &mut fx.debug_context {
-        debug_context.source_locations_enabled = true;
-    }
-
     let rpo = traversal::reverse_postorder(&mir_body);
     let mut visited = BitSet::new_empty(mir_body.basic_blocks().len());
 

src/librustc_codegen_ssa/traits/debuginfo.rs

@@ -49,7 +49,6 @@ pub trait DebugInfoBuilderMethods: BackendTypes {
     // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.).
     fn dbg_var_addr(
         &mut self,
-        dbg_context: &FunctionDebugContext<Self::DIScope>,
         dbg_var: Self::DIVariable,
         scope_metadata: Self::DIScope,
         variable_alloca: Self::Value,
@@ -58,12 +57,7 @@ pub trait DebugInfoBuilderMethods: BackendTypes {
         indirect_offsets: &[Size],
         span: Span,
     );
-    fn set_source_location(
-        &mut self,
-        debug_context: &mut FunctionDebugContext<Self::DIScope>,
-        scope: Self::DIScope,
-        span: Span,
-    );
+    fn set_source_location(&mut self, scope: Self::DIScope, span: Span);
     fn insert_reference_to_gdb_debug_scripts_section_global(&mut self);
     fn set_var_name(&mut self, value: Self::Value, name: &str);
 }