Skip to content

Commit ccff2ba

Browse files
committed
Null-terminate location file names
This saves 8 bytes in `Location` and allows for deduplicating file names more effectively through the linker in the future (by placing them in special seections).
1 parent 650991d commit ccff2ba

File tree

4 files changed

+100
-18
lines changed

4 files changed

+100
-18
lines changed

compiler/rustc_const_eval/src/interpret/intern.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ pub fn intern_const_alloc_recursive<
393393
debug!(?todo);
394394
debug!("dead_alloc_map: {:#?}", ecx.memory.dead_alloc_map);
395395
while let Some(alloc_id) = todo.pop() {
396-
if let Some((_, mut alloc)) = ecx.memory.alloc_map.remove(&alloc_id) {
396+
if let Some((alloc_kind, mut alloc)) = ecx.memory.alloc_map.remove(&alloc_id) {
397397
// We can't call the `intern_shallow` method here, as its logic is tailored to safe
398398
// references and a `leftover_allocations` set (where we only have a todo-list here).
399399
// So we hand-roll the interning logic here again.
@@ -424,9 +424,12 @@ pub fn intern_const_alloc_recursive<
424424
// something that cannot be promoted, which in constants means values that have
425425
// drop glue, such as the example above.
426426
InternKind::Constant => {
427-
ecx.tcx.sess.emit_err(UnsupportedUntypedPointer { span: ecx.tcx.span });
428-
// For better errors later, mark the allocation as immutable.
429-
alloc.mutability = Mutability::Not;
427+
// The Location does have leftover allocations, but that's fine, as we control it.
428+
if alloc_kind != MemoryKind::CallerLocation {
429+
ecx.tcx.sess.emit_err(UnsupportedUntypedPointer { span: ecx.tcx.span });
430+
// For better errors later, mark the allocation as immutable.
431+
alloc.mutability = Mutability::Not;
432+
}
430433
}
431434
}
432435
let alloc = tcx.mk_const_alloc(alloc);

compiler/rustc_const_eval/src/util/caller_location.rs

+14-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use rustc_middle::mir;
33
use rustc_middle::ty::layout::LayoutOf;
44
use rustc_middle::ty::{self, TyCtxt};
55
use rustc_span::{source_map::DUMMY_SP, symbol::Symbol};
6+
use rustc_target::abi::Align;
67
use rustc_type_ir::Mutability;
78

89
use crate::const_eval::{mk_eval_cx, CanAccessStatics, CompileTimeEvalContext};
@@ -19,13 +20,23 @@ fn alloc_caller_location<'mir, 'tcx>(
1920
// This can fail if rustc runs out of memory right here. Trying to emit an error would be
2021
// pointless, since that would require allocating more memory than these short strings.
2122
let file = if loc_details.file {
22-
ecx.allocate_str(filename.as_str(), MemoryKind::CallerLocation, Mutability::Not).unwrap()
23+
let mut bytes = filename.as_str().to_owned().into_bytes();
24+
bytes.push(0);
25+
ecx.allocate_bytes_ptr(&bytes, Align::ONE, MemoryKind::CallerLocation, Mutability::Not)
26+
.unwrap()
2327
} else {
2428
// FIXME: This creates a new allocation each time. It might be preferable to
2529
// perform this allocation only once, and re-use the `MPlaceTy`.
2630
// See https://github.com./rust-lang/rust/pull/89920#discussion_r730012398
27-
ecx.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not).unwrap()
31+
ecx.allocate_bytes_ptr(
32+
b"<redacted>\0",
33+
Align::ONE,
34+
MemoryKind::CallerLocation,
35+
Mutability::Not,
36+
)
37+
.unwrap()
2838
};
39+
let file = Scalar::from_pointer(file, ecx);
2940
let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) };
3041
let col = if loc_details.column { Scalar::from_u32(col) } else { Scalar::from_u32(0) };
3142

@@ -38,7 +49,7 @@ fn alloc_caller_location<'mir, 'tcx>(
3849
let location = ecx.allocate(loc_layout, MemoryKind::CallerLocation).unwrap();
3950

4051
// Initialize fields.
41-
ecx.write_immediate(file.to_ref(ecx), &ecx.project_field(&location, 0).unwrap())
52+
ecx.write_scalar(file, &ecx.project_field(&location, 0).unwrap())
4253
.expect("writing to memory we just allocated cannot fail");
4354
ecx.write_scalar(line, &ecx.project_field(&location, 1).unwrap())
4455
.expect("writing to memory we just allocated cannot fail");

library/core/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
#![feature(const_caller_location)]
127127
#![feature(const_cell_into_inner)]
128128
#![feature(const_char_from_u32_unchecked)]
129+
#![feature(const_cstr_from_ptr)]
129130
#![feature(const_eval_select)]
130131
#![feature(const_exact_div)]
131132
#![feature(const_float_bits_conv)]

library/core/src/panic/location.rs

+78-11
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
use crate::fmt;
2+
#[cfg(not(bootstrap))]
3+
use crate::{ffi::CStr, marker::PhantomData};
24

35
/// A struct containing information about the location of a panic.
46
///
@@ -28,8 +30,21 @@ use crate::fmt;
2830
/// Files are compared as strings, not `Path`, which could be unexpected.
2931
/// See [`Location::file`]'s documentation for more discussion.
3032
#[lang = "panic_location"]
33+
#[derive(Copy, Clone)]
34+
#[stable(feature = "panic_hooks", since = "1.10.0")]
35+
#[cfg(not(bootstrap))]
36+
pub struct Location<'a> {
37+
file: *const (),
38+
_file_actual: PhantomData<&'a CStr>,
39+
line: u32,
40+
col: u32,
41+
}
42+
43+
/// Location
44+
#[lang = "panic_location"]
3145
#[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
3246
#[stable(feature = "panic_hooks", since = "1.10.0")]
47+
#[cfg(bootstrap)]
3348
pub struct Location<'a> {
3449
file: &'a str,
3550
line: u32,
@@ -126,7 +141,17 @@ impl<'a> Location<'a> {
126141
#[rustc_const_unstable(feature = "const_location_fields", issue = "102911")]
127142
#[inline]
128143
pub const fn file(&self) -> &str {
129-
self.file
144+
#[cfg(bootstrap)]
145+
{
146+
self.file
147+
}
148+
#[cfg(not(bootstrap))]
149+
// SAFETY: nyaaa
150+
unsafe {
151+
let cstr = CStr::from_ptr(self.file as _);
152+
let len = cstr.count_bytes();
153+
crate::str::from_utf8_unchecked(crate::slice::from_raw_parts(self.file as _, len))
154+
}
130155
}
131156

132157
/// Returns the line number from which the panic originated.
@@ -180,21 +205,63 @@ impl<'a> Location<'a> {
180205
}
181206
}
182207

183-
#[unstable(
184-
feature = "panic_internals",
185-
reason = "internal details of the implementation of the `panic!` and related macros",
186-
issue = "none"
187-
)]
188-
impl<'a> Location<'a> {
189-
#[doc(hidden)]
190-
pub const fn internal_constructor(file: &'a str, line: u32, col: u32) -> Self {
191-
Location { file, line, col }
208+
// cfg(not(bootstrap)) NOTE: inline this module when bumping, it's only here to group the cfg
209+
#[cfg(not(bootstrap))]
210+
mod location_impls {
211+
use super::Location;
212+
213+
impl Location<'_> {
214+
fn to_parts(&self) -> (&str, u32, u32) {
215+
(self.file(), self.line, self.col)
216+
}
217+
}
218+
219+
#[stable(feature = "panic_hooks", since = "1.10.0")]
220+
impl crate::fmt::Debug for Location<'_> {
221+
fn fmt(&self, f: &mut crate::fmt::Formatter<'_>) -> crate::fmt::Result {
222+
f.debug_struct("Location")
223+
.field("file", &self.file())
224+
.field("line", &self.line)
225+
.field("col", &self.col)
226+
.finish()
227+
}
228+
}
229+
230+
#[stable(feature = "panic_hooks", since = "1.10.0")]
231+
impl crate::cmp::PartialEq for Location<'_> {
232+
fn eq(&self, other: &Self) -> bool {
233+
self.to_parts() == other.to_parts()
234+
}
235+
}
236+
237+
#[stable(feature = "panic_hooks", since = "1.10.0")]
238+
impl crate::cmp::Eq for Location<'_> {}
239+
240+
#[stable(feature = "panic_hooks", since = "1.10.0")]
241+
impl crate::cmp::PartialOrd for Location<'_> {
242+
fn partial_cmp(&self, other: &Self) -> Option<crate::cmp::Ordering> {
243+
self.to_parts().partial_cmp(&other.to_parts())
244+
}
245+
}
246+
247+
#[stable(feature = "panic_hooks", since = "1.10.0")]
248+
impl crate::cmp::Ord for Location<'_> {
249+
fn cmp(&self, other: &Self) -> crate::cmp::Ordering {
250+
self.to_parts().cmp(&other.to_parts())
251+
}
252+
}
253+
254+
#[stable(feature = "panic_hooks", since = "1.10.0")]
255+
impl crate::hash::Hash for Location<'_> {
256+
fn hash<H: crate::hash::Hasher>(&self, state: &mut H) {
257+
self.to_parts().hash(state)
258+
}
192259
}
193260
}
194261

195262
#[stable(feature = "panic_hook_display", since = "1.26.0")]
196263
impl fmt::Display for Location<'_> {
197264
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
198-
write!(formatter, "{}:{}:{}", self.file, self.line, self.col)
265+
write!(formatter, "{}:{}:{}", self.file(), self.line, self.col)
199266
}
200267
}

0 commit comments

Comments
 (0)