Auto merge of rust-lang#117431 - Nilstrieb:nul-location, r=<try>

Null terminate `core::panic::Location` file strings

This shrinks `Location` by a usize, making it 24->16 bytes on x86-64. This reduces binary size. I quickly measure `cargo install ripgrep`, which was reduced by 0.3%. That's not a lot, but it's something and the code change is quite simple. From some quick measurements, I expect librustc_driver to shrink by more, but rustc-perf should give us more numbers.

The change to const interning is necessary, see https://rust-lang.zulipchat.com/#narrow/stream/146212-t-compiler.2Fconst-eval/topic/unsupported.20untyped.20pointer.20in.20constant.20error/near/399446285

Author: bors

compiler/rustc_const_eval/src/interpret/intern.rs

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

compiler/rustc_const_eval/src/util/caller_location.rs

@@ -3,6 +3,7 @@ use rustc_middle::mir;
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::{source_map::DUMMY_SP, symbol::Symbol};
+use rustc_target::abi::Align;
 use rustc_type_ir::Mutability;
 
 use crate::const_eval::{mk_eval_cx, CanAccessStatics, CompileTimeEvalContext};
@@ -19,13 +20,23 @@ fn alloc_caller_location<'mir, 'tcx>(
     // This can fail if rustc runs out of memory right here. Trying to emit an error would be
     // pointless, since that would require allocating more memory than these short strings.
     let file = if loc_details.file {
-        ecx.allocate_str(filename.as_str(), MemoryKind::CallerLocation, Mutability::Not).unwrap()
+        let mut bytes = filename.as_str().to_owned().into_bytes();
+        bytes.push(0);
+        ecx.allocate_bytes_ptr(&bytes, Align::ONE, MemoryKind::CallerLocation, Mutability::Not)
+            .unwrap()
     } else {
         // FIXME: This creates a new allocation each time. It might be preferable to
         // perform this allocation only once, and re-use the `MPlaceTy`.
         // See https://github.com/rust-lang/rust/pull/89920#discussion_r730012398
-        ecx.allocate_str("<redacted>", MemoryKind::CallerLocation, Mutability::Not).unwrap()
+        ecx.allocate_bytes_ptr(
+            b"<redacted>\0",
+            Align::ONE,
+            MemoryKind::CallerLocation,
+            Mutability::Not,
+        )
+        .unwrap()
     };
+    let file = Scalar::from_pointer(file, ecx);
     let line = if loc_details.line { Scalar::from_u32(line) } else { Scalar::from_u32(0) };
     let col = if loc_details.column { Scalar::from_u32(col) } else { Scalar::from_u32(0) };
 
@@ -38,11 +49,11 @@ fn alloc_caller_location<'mir, 'tcx>(
     let location = ecx.allocate(loc_layout, MemoryKind::CallerLocation).unwrap();
 
     // Initialize fields.
-    ecx.write_immediate(file.to_ref(ecx), &ecx.project_field(&location, 0).unwrap())
+    ecx.write_scalar(file, &ecx.project_field(&location, 0).unwrap())
         .expect("writing to memory we just allocated cannot fail");
-    ecx.write_scalar(line, &ecx.project_field(&location, 1).unwrap())
+    ecx.write_scalar(line, &ecx.project_field(&location, 2).unwrap())
         .expect("writing to memory we just allocated cannot fail");
-    ecx.write_scalar(col, &ecx.project_field(&location, 2).unwrap())
+    ecx.write_scalar(col, &ecx.project_field(&location, 3).unwrap())
         .expect("writing to memory we just allocated cannot fail");
 
     location

library/core/src/lib.rs

@@ -126,6 +126,7 @@
 #![feature(const_caller_location)]
 #![feature(const_cell_into_inner)]
 #![feature(const_char_from_u32_unchecked)]
+#![feature(const_cstr_from_ptr)]
 #![feature(const_eval_select)]
 #![feature(const_exact_div)]
 #![feature(const_float_bits_conv)]

library/core/src/panic/location.rs

@@ -1,4 +1,6 @@
 use crate::fmt;
+#[cfg(not(bootstrap))]
+use crate::{ffi::CStr, marker::PhantomData};
 
 /// A struct containing information about the location of a panic.
 ///
@@ -28,8 +30,21 @@ use crate::fmt;
 /// Files are compared as strings, not `Path`, which could be unexpected.
 /// See [`Location::file`]'s documentation for more discussion.
 #[lang = "panic_location"]
+#[derive(Copy, Clone)]
+#[stable(feature = "panic_hooks", since = "1.10.0")]
+#[cfg(not(bootstrap))]
+pub struct Location<'a> {
+    file: *const (),
+    _file_actual: PhantomData<&'a CStr>,
+    line: u32,
+    col: u32,
+}
+
+/// Location
+#[lang = "panic_location"]
 #[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
 #[stable(feature = "panic_hooks", since = "1.10.0")]
+#[cfg(bootstrap)]
 pub struct Location<'a> {
     file: &'a str,
     line: u32,
@@ -126,7 +141,17 @@ impl<'a> Location<'a> {
     #[rustc_const_unstable(feature = "const_location_fields", issue = "102911")]
     #[inline]
     pub const fn file(&self) -> &str {
-        self.file
+        #[cfg(bootstrap)]
+        {
+            self.file
+        }
+        #[cfg(not(bootstrap))]
+        // SAFETY: Codegen ensures that there is a null byte.
+        unsafe {
+            let cstr = CStr::from_ptr(self.file as _);
+            let len = cstr.count_bytes();
+            crate::str::from_utf8_unchecked(crate::slice::from_raw_parts(self.file as _, len))
+        }
     }
 
     /// Returns the line number from which the panic originated.
@@ -180,21 +205,63 @@ impl<'a> Location<'a> {
     }
 }
 
-#[unstable(
-    feature = "panic_internals",
-    reason = "internal details of the implementation of the `panic!` and related macros",
-    issue = "none"
-)]
-impl<'a> Location<'a> {
-    #[doc(hidden)]
-    pub const fn internal_constructor(file: &'a str, line: u32, col: u32) -> Self {
-        Location { file, line, col }
+// cfg(not(bootstrap)) NOTE: inline this module when bumping, it's only here to group the cfg
+#[cfg(not(bootstrap))]
+mod location_impls {
+    use super::Location;
+
+    impl Location<'_> {
+        fn to_parts(&self) -> (&str, u32, u32) {
+            (self.file(), self.line, self.col)
+        }
+    }
+
+    #[stable(feature = "panic_hooks", since = "1.10.0")]
+    impl crate::fmt::Debug for Location<'_> {
+        fn fmt(&self, f: &mut crate::fmt::Formatter<'_>) -> crate::fmt::Result {
+            f.debug_struct("Location")
+                .field("file", &self.file())
+                .field("line", &self.line)
+                .field("col", &self.col)
+                .finish()
+        }
+    }
+
+    #[stable(feature = "panic_hooks", since = "1.10.0")]
+    impl crate::cmp::PartialEq for Location<'_> {
+        fn eq(&self, other: &Self) -> bool {
+            self.to_parts() == other.to_parts()
+        }
+    }
+
+    #[stable(feature = "panic_hooks", since = "1.10.0")]
+    impl crate::cmp::Eq for Location<'_> {}
+
+    #[stable(feature = "panic_hooks", since = "1.10.0")]
+    impl crate::cmp::PartialOrd for Location<'_> {
+        fn partial_cmp(&self, other: &Self) -> Option<crate::cmp::Ordering> {
+            self.to_parts().partial_cmp(&other.to_parts())
+        }
+    }
+
+    #[stable(feature = "panic_hooks", since = "1.10.0")]
+    impl crate::cmp::Ord for Location<'_> {
+        fn cmp(&self, other: &Self) -> crate::cmp::Ordering {
+            self.to_parts().cmp(&other.to_parts())
+        }
+    }
+
+    #[stable(feature = "panic_hooks", since = "1.10.0")]
+    impl crate::hash::Hash for Location<'_> {
+        fn hash<H: crate::hash::Hasher>(&self, state: &mut H) {
+            self.to_parts().hash(state)
+        }
     }
 }
 
 #[stable(feature = "panic_hook_display", since = "1.26.0")]
 impl fmt::Display for Location<'_> {
     fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(formatter, "{}:{}:{}", self.file, self.line, self.col)
+        write!(formatter, "{}:{}:{}", self.file(), self.line, self.col)
     }
 }