Auto merge of #150463 - JonathanBrouwer:rollup-ietend4, r=JonathanBrouwer

Rollup of 5 pull requests

Successful merges:

 - #147499 (Implement round-ties-to-even for Duration Debug for consistency with f64)
 - #149447 (Rewrite `String::replace_range`)
 - #149469 (Leverage &mut in OnceLock when possible)
 - #149921 (Add new source component that includes GPL-licensed source)
 - #150460 (fix ManuallyDrop::into_inner aliasing (Miri) issues)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

library/alloc/src/string.rs

@@ -50,8 +50,6 @@ use core::iter::from_fn;
 use core::ops::Add;
 #[cfg(not(no_global_oom_handling))]
 use core::ops::AddAssign;
-#[cfg(not(no_global_oom_handling))]
-use core::ops::Bound::{Excluded, Included, Unbounded};
 use core::ops::{self, Range, RangeBounds};
 use core::str::pattern::{Pattern, Utf8Pattern};
 use core::{fmt, hash, ptr, slice};
@@ -2059,30 +2057,19 @@ impl String {
     where
         R: RangeBounds<usize>,
     {
-        // Memory safety
-        //
-        // Replace_range does not have the memory safety issues of a vector Splice.
-        // of the vector version. The data is just plain bytes.
-
-        // WARNING: Inlining this variable would be unsound (#81138)
-        let start = range.start_bound();
-        match start {
-            Included(&n) => assert!(self.is_char_boundary(n)),
-            Excluded(&n) => assert!(self.is_char_boundary(n + 1)),
-            Unbounded => {}
-        };
-        // WARNING: Inlining this variable would be unsound (#81138)
-        let end = range.end_bound();
-        match end {
-            Included(&n) => assert!(self.is_char_boundary(n + 1)),
-            Excluded(&n) => assert!(self.is_char_boundary(n)),
-            Unbounded => {}
-        };
-
-        // Using `range` again would be unsound (#81138)
-        // We assume the bounds reported by `range` remain the same, but
-        // an adversarial implementation could change between calls
-        unsafe { self.as_mut_vec() }.splice((start, end), replace_with.bytes());
+        // We avoid #81138 (nondeterministic RangeBounds impls) because we only use `range` once, here.
+        let checked_range = slice::range(range, ..self.len());
+
+        assert!(
+            self.is_char_boundary(checked_range.start),
+            "start of range should be a character boundary"
+        );
+        assert!(
+            self.is_char_boundary(checked_range.end),
+            "end of range should be a character boundary"
+        );
+
+        unsafe { self.as_mut_vec() }.splice(checked_range, replace_with.bytes());
     }
 
     /// Replaces the leftmost occurrence of a pattern with another string, in-place.

library/alloctests/tests/string.rs

@@ -616,8 +616,15 @@ fn test_replace_range() {
 }
 
 #[test]
-#[should_panic]
-fn test_replace_range_char_boundary() {
+#[should_panic = "start of range should be a character boundary"]
+fn test_replace_range_start_char_boundary() {
+    let mut s = "Hello, 世界!".to_owned();
+    s.replace_range(8.., "");
+}
+
+#[test]
+#[should_panic = "end of range should be a character boundary"]
+fn test_replace_range_end_char_boundary() {
     let mut s = "Hello, 世界!".to_owned();
     s.replace_range(..8, "");
 }
@@ -632,28 +639,32 @@ fn test_replace_range_inclusive_range() {
 }
 
 #[test]
-#[should_panic]
+#[should_panic = "range end index 6 out of range for slice of length 5"]
 fn test_replace_range_out_of_bounds() {
     let mut s = String::from("12345");
     s.replace_range(5..6, "789");
 }
 
 #[test]
-#[should_panic]
+#[should_panic = "range end index 5 out of range for slice of length 5"]
 fn test_replace_range_inclusive_out_of_bounds() {
     let mut s = String::from("12345");
     s.replace_range(5..=5, "789");
 }
 
+// The overflowed index value is target-dependent,
+// so we don't check for its exact value in the panic message
 #[test]
-#[should_panic]
+#[should_panic = "out of range for slice of length 3"]
 fn test_replace_range_start_overflow() {
     let mut s = String::from("123");
     s.replace_range((Excluded(usize::MAX), Included(0)), "");
 }
 
+// The overflowed index value is target-dependent,
+// so we don't check for its exact value in the panic message
 #[test]
-#[should_panic]
+#[should_panic = "out of range for slice of length 3"]
 fn test_replace_range_end_overflow() {
     let mut s = String::from("456");
     s.replace_range((Included(0), Included(usize::MAX)), "");

library/core/src/mem/manually_drop.rs

@@ -200,7 +200,9 @@ impl<T> ManuallyDrop<T> {
     #[rustc_const_stable(feature = "const_manually_drop", since = "1.32.0")]
     #[inline(always)]
     pub const fn into_inner(slot: ManuallyDrop<T>) -> T {
-        slot.value.into_inner()
+        // Cannot use `MaybeDangling::into_inner` as that does not yet have the desired semantics.
+        // SAFETY: We know this is a valid `T`. `slot` will not be dropped.
+        unsafe { (&raw const slot).cast::<T>().read() }
     }
 
     /// Takes the value from the `ManuallyDrop<T>` container out.

library/core/src/time.rs

@@ -1315,39 +1315,54 @@ impl fmt::Debug for Duration {
             // need to perform rounding to match the semantics of printing
             // normal floating point numbers. However, we only need to do work
             // when rounding up. This happens if the first digit of the
-            // remaining ones is >= 5.
+            // remaining ones is >= 5. When the first digit is exactly 5, rounding
+            // follows IEEE-754 round-ties-to-even semantics: we only round up
+            // if the last written digit is odd.
             let integer_part = if fractional_part > 0 && fractional_part >= divisor * 5 {
-                // Round up the number contained in the buffer. We go through
-                // the buffer backwards and keep track of the carry.
-                let mut rev_pos = pos;
-                let mut carry = true;
-                while carry && rev_pos > 0 {
-                    rev_pos -= 1;
-
-                    // If the digit in the buffer is not '9', we just need to
-                    // increment it and can stop then (since we don't have a
-                    // carry anymore). Otherwise, we set it to '0' (overflow)
-                    // and continue.
-                    if buf[rev_pos] < b'9' {
-                        buf[rev_pos] += 1;
-                        carry = false;
-                    } else {
-                        buf[rev_pos] = b'0';
-                    }
-                }
-
-                // If we still have the carry bit set, that means that we set
-                // the whole buffer to '0's and need to increment the integer
-                // part.
-                if carry {
-                    // If `integer_part == u64::MAX` and precision < 9, any
-                    // carry of the overflow during rounding of the
-                    // `fractional_part` into the `integer_part` will cause the
-                    // `integer_part` itself to overflow. Avoid this by using an
-                    // `Option<u64>`, with `None` representing `u64::MAX + 1`.
-                    integer_part.checked_add(1)
+                // For ties (fractional_part == divisor * 5), only round up if last digit is odd
+                let is_tie = fractional_part == divisor * 5;
+                let last_digit_is_odd = if pos > 0 {
+                    (buf[pos - 1] - b'0') % 2 == 1
                 } else {
+                    // No fractional digits - check the integer part
+                    (integer_part % 2) == 1
+                };
+
+                if is_tie && !last_digit_is_odd {
                     Some(integer_part)
+                } else {
+                    // Round up the number contained in the buffer. We go through
+                    // the buffer backwards and keep track of the carry.
+                    let mut rev_pos = pos;
+                    let mut carry = true;
+                    while carry && rev_pos > 0 {
+                        rev_pos -= 1;
+
+                        // If the digit in the buffer is not '9', we just need to
+                        // increment it and can stop then (since we don't have a
+                        // carry anymore). Otherwise, we set it to '0' (overflow)
+                        // and continue.
+                        if buf[rev_pos] < b'9' {
+                            buf[rev_pos] += 1;
+                            carry = false;
+                        } else {
+                            buf[rev_pos] = b'0';
+                        }
+                    }
+
+                    // If we still have the carry bit set, that means that we set
+                    // the whole buffer to '0's and need to increment the integer
+                    // part.
+                    if carry {
+                        // If `integer_part == u64::MAX` and precision < 9, any
+                        // carry of the overflow during rounding of the
+                        // `fractional_part` into the `integer_part` will cause the
+                        // `integer_part` itself to overflow. Avoid this by using an
+                        // `Option<u64>`, with `None` representing `u64::MAX + 1`.
+                        integer_part.checked_add(1)
+                    } else {
+                        Some(integer_part)
+                    }
                 }
             } else {
                 Some(integer_part)

library/coretests/tests/time.rs

@@ -439,7 +439,6 @@ fn debug_formatting_precision_two() {
     assert_eq!(format!("{:.2?}", Duration::new(4, 001_000_000)), "4.00s");
     assert_eq!(format!("{:.2?}", Duration::new(2, 100_000_000)), "2.10s");
     assert_eq!(format!("{:.2?}", Duration::new(2, 104_990_000)), "2.10s");
-    assert_eq!(format!("{:.2?}", Duration::new(2, 105_000_000)), "2.11s");
     assert_eq!(format!("{:.2?}", Duration::new(8, 999_999_999)), "9.00s");
 }
 
@@ -480,6 +479,15 @@ fn debug_formatting_precision_high() {
     assert_eq!(format!("{:.20?}", Duration::new(4, 001_000_000)), "4.00100000000000000000s");
 }
 
+#[test]
+fn debug_formatting_round_to_even() {
+    assert_eq!(format!("{:.0?}", Duration::new(1, 500_000_000)), "2s");
+    assert_eq!(format!("{:.0?}", Duration::new(2, 500_000_000)), "2s");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 1_500_000)), "2ms");
+    assert_eq!(format!("{:.0?}", Duration::new(0, 2_500_000)), "2ms");
+    assert_eq!(format!("{:.2?}", Duration::new(2, 105_000_000)), "2.10s");
+}
+
 #[test]
 fn duration_const() {
     // test that the methods of `Duration` are usable in a const context

library/std/src/sync/once_lock.rs

@@ -1,3 +1,4 @@
+use super::once::OnceExclusiveState;
 use crate::cell::UnsafeCell;
 use crate::fmt;
 use crate::marker::PhantomData;
@@ -152,8 +153,8 @@ impl<T> OnceLock<T> {
     #[stable(feature = "once_cell", since = "1.70.0")]
     #[rustc_should_not_be_called_on_const_items]
     pub fn get(&self) -> Option<&T> {
-        if self.is_initialized() {
-            // Safe b/c checked is_initialized
+        if self.initialized() {
+            // Safe b/c checked initialized
             Some(unsafe { self.get_unchecked() })
         } else {
             None
@@ -170,8 +171,8 @@ impl<T> OnceLock<T> {
     #[inline]
     #[stable(feature = "once_cell", since = "1.70.0")]
     pub fn get_mut(&mut self) -> Option<&mut T> {
-        if self.is_initialized() {
-            // Safe b/c checked is_initialized and we have a unique access
+        if self.initialized_mut() {
+            // Safe b/c checked initialized and we have a unique access
             Some(unsafe { self.get_unchecked_mut() })
         } else {
             None
@@ -402,14 +403,12 @@ impl<T> OnceLock<T> {
         // NOTE: We need to perform an acquire on the state in this method
         // in order to correctly synchronize `LazyLock::force`. This is
         // currently done by calling `self.get()`, which in turn calls
-        // `self.is_initialized()`, which in turn performs the acquire.
+        // `self.initialized()`, which in turn performs the acquire.
         if let Some(value) = self.get() {
             return Ok(value);
         }
         self.initialize(f)?;
 
-        debug_assert!(self.is_initialized());
-
         // SAFETY: The inner value has been initialized
         Ok(unsafe { self.get_unchecked() })
     }
@@ -451,10 +450,10 @@ impl<T> OnceLock<T> {
     where
         F: FnOnce() -> Result<T, E>,
     {
-        if self.get().is_none() {
+        if self.get_mut().is_none() {
             self.initialize(f)?;
         }
-        debug_assert!(self.is_initialized());
+
         // SAFETY: The inner value has been initialized
         Ok(unsafe { self.get_unchecked_mut() })
     }
@@ -503,22 +502,32 @@ impl<T> OnceLock<T> {
     #[inline]
     #[stable(feature = "once_cell", since = "1.70.0")]
     pub fn take(&mut self) -> Option<T> {
-        if self.is_initialized() {
+        if self.initialized_mut() {
             self.once = Once::new();
             // SAFETY: `self.value` is initialized and contains a valid `T`.
-            // `self.once` is reset, so `is_initialized()` will be false again
+            // `self.once` is reset, so `initialized()` will be false again
             // which prevents the value from being read twice.
-            unsafe { Some((&mut *self.value.get()).assume_init_read()) }
+            unsafe { Some(self.value.get_mut().assume_init_read()) }
         } else {
             None
         }
     }
 
     #[inline]
-    fn is_initialized(&self) -> bool {
+    fn initialized(&self) -> bool {
         self.once.is_completed()
     }
 
+    #[inline]
+    fn initialized_mut(&mut self) -> bool {
+        // `state()` does not perform an atomic load, so prefer it over `is_complete()`.
+        let state = self.once.state();
+        match state {
+            OnceExclusiveState::Complete => true,
+            _ => false,
+        }
+    }
+
     #[cold]
     #[optimize(size)]
     fn initialize<F, E>(&self, f: F) -> Result<(), E>
@@ -552,7 +561,7 @@ impl<T> OnceLock<T> {
     /// The cell must be initialized
     #[inline]
     unsafe fn get_unchecked(&self) -> &T {
-        debug_assert!(self.is_initialized());
+        debug_assert!(self.initialized());
         unsafe { (&*self.value.get()).assume_init_ref() }
     }
 
@@ -561,8 +570,8 @@ impl<T> OnceLock<T> {
     /// The cell must be initialized
     #[inline]
     unsafe fn get_unchecked_mut(&mut self) -> &mut T {
-        debug_assert!(self.is_initialized());
-        unsafe { (&mut *self.value.get()).assume_init_mut() }
+        debug_assert!(self.initialized_mut());
+        unsafe { self.value.get_mut().assume_init_mut() }
     }
 }
 
@@ -690,11 +699,11 @@ impl<T: Eq> Eq for OnceLock<T> {}
 unsafe impl<#[may_dangle] T> Drop for OnceLock<T> {
     #[inline]
     fn drop(&mut self) {
-        if self.is_initialized() {
+        if self.initialized_mut() {
             // SAFETY: The cell is initialized and being dropped, so it can't
             // be accessed again. We also don't touch the `T` other than
             // dropping it, which validates our usage of #[may_dangle].
-            unsafe { (&mut *self.value.get()).assume_init_drop() };
+            unsafe { self.value.get_mut().assume_init_drop() };
         }
     }
 }