Skip to content

Commit

Permalink
Auto merge of rust-lang#82285 - nhwn:nonzero-debug, r=nagisa
Browse files Browse the repository at this point in the history
Use u32 over Option<u32> in DebugLoc

~~Changes `Option<u32>` fields in `DebugLoc` to `Option<NonZeroU32>`. Since the respective fields (`line` and `col`) are guaranteed to be 1-based, this layout optimization is a freebie.~~

EDIT: Changes `Option<u32>` fields in `DebugLoc` to `u32`. As `@bugadani` pointed out, an `Option<NonZeroU32>` is probably an unnecessary layer of abstraction since the `None` variant is always used as `UNKNOWN_LINE_NUMBER` (which is just `0`).  Also, `SourceInfo` in `metadata.rs` already uses a `u32` instead of an `Option<u32>` to encode the same information, so I think this change is warranted.

Since `@jyn514` raised some concerns over measuring performance in a similar PR (rust-lang#82255), does this need a perf run?
  • Loading branch information
bors committed Mar 7, 2021
2 parents 66ec64c + 408d402 commit 234781a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 25 deletions.
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::metadata::{file_metadata, UNKNOWN_COLUMN_NUMBER, UNKNOWN_LINE_NUMBER};
use super::metadata::file_metadata;
use super::utils::DIB;
use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext};
use rustc_codegen_ssa::traits::*;
Expand Down Expand Up @@ -102,8 +102,8 @@ fn make_mir_scope(
DIB(cx),
parent_scope.dbg_scope.unwrap(),
file_metadata,
loc.line.unwrap_or(UNKNOWN_LINE_NUMBER),
loc.col.unwrap_or(UNKNOWN_COLUMN_NUMBER),
loc.line,
loc.col,
)
},
};
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1842,10 +1842,7 @@ impl<'tcx> VariantInfo<'_, 'tcx> {
.span;
if !span.is_dummy() {
let loc = cx.lookup_debug_loc(span.lo());
return Some(SourceInfo {
file: file_metadata(cx, &loc.file),
line: loc.line.unwrap_or(UNKNOWN_LINE_NUMBER),
});
return Some(SourceInfo { file: file_metadata(cx, &loc.file), line: loc.line });
}
}
_ => {}
Expand Down Expand Up @@ -2484,7 +2481,7 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global
let loc = cx.lookup_debug_loc(span.lo());
(file_metadata(cx, &loc.file), loc.line)
} else {
(unknown_file_metadata(cx), None)
(unknown_file_metadata(cx), UNKNOWN_LINE_NUMBER)
};

let is_local_to_unit = is_node_local_to_unit(cx, def_id);
Expand All @@ -2507,7 +2504,7 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global
linkage_name.as_ptr().cast(),
linkage_name.len(),
file_metadata,
line_number.unwrap_or(UNKNOWN_LINE_NUMBER),
line_number,
type_metadata,
is_local_to_unit,
global,
Expand Down
25 changes: 9 additions & 16 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ pub struct DebugLoc {
/// Information about the original source file.
pub file: Lrc<SourceFile>,
/// The (1-based) line number.
pub line: Option<u32>,
pub line: u32,
/// The (1-based) column number.
pub col: Option<u32>,
pub col: u32,
}

impl CodegenCx<'ll, '_> {
Expand All @@ -243,16 +243,16 @@ impl CodegenCx<'ll, '_> {
let line = (line + 1) as u32;
let col = (pos - line_pos).to_u32() + 1;

(file, Some(line), Some(col))
(file, line, col)
}
Err(file) => (file, None, None),
Err(file) => (file, UNKNOWN_LINE_NUMBER, UNKNOWN_COLUMN_NUMBER),
};

// For MSVC, omit the column number.
// Otherwise, emit it. This mimics clang behaviour.
// See discussion in https://github.com/rust-lang/rust/issues/42921
if self.sess().target.is_like_msvc {
DebugLoc { file, line, col: None }
DebugLoc { file, line, col: UNKNOWN_COLUMN_NUMBER }
} else {
DebugLoc { file, line, col }
}
Expand Down Expand Up @@ -358,9 +358,9 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
linkage_name.as_ptr().cast(),
linkage_name.len(),
file_metadata,
loc.line.unwrap_or(UNKNOWN_LINE_NUMBER),
loc.line,
function_type_metadata,
scope_line.unwrap_or(UNKNOWN_LINE_NUMBER),
scope_line,
flags,
spflags,
maybe_definition_llfn,
Expand Down Expand Up @@ -550,14 +550,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
) -> &'ll DILocation {
let DebugLoc { line, col, .. } = self.lookup_debug_loc(span.lo());

unsafe {
llvm::LLVMRustDIBuilderCreateDebugLocation(
line.unwrap_or(UNKNOWN_LINE_NUMBER),
col.unwrap_or(UNKNOWN_COLUMN_NUMBER),
scope,
inlined_at,
)
}
unsafe { llvm::LLVMRustDIBuilderCreateDebugLocation(line, col, scope, inlined_at) }
}

fn create_vtable_metadata(&self, ty: Ty<'tcx>, vtable: Self::Value) {
Expand Down Expand Up @@ -606,7 +599,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
name.as_ptr().cast(),
name.len(),
file_metadata,
loc.line.unwrap_or(UNKNOWN_LINE_NUMBER),
loc.line,
type_metadata,
true,
DIFlags::FlagZero,
Expand Down

0 comments on commit 234781a

Please sign in to comment.