Unrolled build for #155858

rust-timer · web-flow · commit 5db57c86f2ab · 2026-04-28T16:57:25.000+02:00
Rollup merge of #155858 - ChrisDenton:borrowed-len, r=jhpratt `slice::join`: borrow only once during length calc This ensures the length calculation will always be self-consistent even if the real length changes when we actually come to use them. This is a follow up to #155708
diff --git a/library/alloc/src/str.rs b/library/alloc/src/str.rs
@@ -135,13 +135,9 @@ macro_rules! copy_slice_and_advance {
 //
 // This implementation calls `borrow()` multiple times:
 // 1. To calculate `reserved_len`, all elements are borrowed once.
-// 2. The first element is borrowed again when copied via `extend_from_slice`.
-// 3. Subsequent elements are borrowed a second time when building the mapped iterator.
+// 2. All elements, except the first, are borrowed a second time when building the mapped iterator.
 //
 // Risks and Mitigations:
-// - If the first element GROWS on the second borrow, the length subtraction underflows.
-//   We mitigate this by doing a `checked_sub` to panic rather than allowing an underflow
-//   that fabricates a huge destination slice.
 // - If elements 2..N GROW on their second borrow, the target slice bounds set by `checked_sub`
 //   means that `split_at_mut` inside `copy_slice_and_advance!` will correctly panic.
 // - If elements SHRINK on their second borrow, the spare space is never written, and the final
@@ -157,8 +153,10 @@ where
     let mut iter = slice.iter();
 
     // the first slice is the only one without a separator preceding it
+    // we take care to only borrow this once during the length calculation
+    // to avoid inconsistent Borrow implementations from breaking our assumptions
     let first = match iter.next() {
-        Some(first) => first,
+        Some(first) => first.borrow().as_ref(),
         None => return vec![],
     };
 
@@ -168,21 +166,24 @@ where
     // the entire Vec pre-allocated for safety
     let reserved_len = sep_len
         .checked_mul(iter.len())
+        .and_then(|n| n.checked_add(first.len()))
         .and_then(|n| {
-            slice.iter().map(|s| s.borrow().as_ref().len()).try_fold(n, usize::checked_add)
+            // iter starts from the second element as we've already taken the first
+            // it's cloned so we can reuse the same iterator below
+            iter.clone().map(|s| s.borrow().as_ref().len()).try_fold(n, usize::checked_add)
         })
         .expect("attempt to join into collection with len > usize::MAX");
 
     // prepare an uninitialized buffer
     let mut result = Vec::with_capacity(reserved_len);
     debug_assert!(result.capacity() >= reserved_len);
 
-    result.extend_from_slice(first.borrow().as_ref());
+    result.extend_from_slice(first);
 
     unsafe {
         let pos = result.len();
-        let target_len = reserved_len.checked_sub(pos).expect("inconsistent Borrow implementation");
-        let target = result.spare_capacity_mut().get_unchecked_mut(..target_len);
+        debug_assert!(reserved_len >= pos);
+        let target = result.spare_capacity_mut().get_unchecked_mut(..reserved_len - pos);
 
         // Convert the separator and slices to slices of MaybeUninit
         // to simplify implementation in specialize_for_lengths.
diff --git a/library/alloctests/tests/str.rs b/library/alloctests/tests/str.rs
@@ -164,7 +164,7 @@ fn test_join_for_different_lengths_with_long_separator() {
 }
 
 #[test]
-fn test_join_issue_80335() {
+fn test_join_inconsistent_borrow_shrink() {
     use core::borrow::Borrow;
     use core::cell::Cell;
 
@@ -191,12 +191,12 @@ fn test_join_issue_80335() {
     }
 
     let arr: [WeirdBorrow; 3] = Default::default();
-    test_join!("0-0-0", arr, "-");
+    test_join!("123456-0-0", arr, "-");
 }
 
 #[test]
-#[should_panic(expected = "inconsistent Borrow implementation")]
-fn test_join_inconsistent_borrow() {
+#[should_panic(expected = "mid > len")]
+fn test_join_inconsistent_borrow_grow() {
     use std::borrow::Borrow;
     use std::cell::Cell;
 

Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ fn test_join_for_different_lengths_with_long_separator() {`
`164`	`164`	`}`
`165`	`165`
`166`	`166`	`#[test]`
`167`		`-fn test_join_issue_80335() {`
	`167`	`+fn test_join_inconsistent_borrow_shrink() {`
`168`	`168`	`use core::borrow::Borrow;`
`169`	`169`	`use core::cell::Cell;`
`170`	`170`
`@@ -191,12 +191,12 @@ fn test_join_issue_80335() {`
`191`	`191`	`}`
`192`	`192`
`193`	`193`	`let arr: [WeirdBorrow; 3] = Default::default();`
`194`		`- test_join!("0-0-0", arr, "-");`
	`194`	`+ test_join!("123456-0-0", arr, "-");`
`195`	`195`	`}`
`196`	`196`
`197`	`197`	`#[test]`
`198`		`-#[should_panic(expected = "inconsistent Borrow implementation")]`
`199`		`-fn test_join_inconsistent_borrow() {`
	`198`	`+#[should_panic(expected = "mid > len")]`
	`199`	`+fn test_join_inconsistent_borrow_grow() {`
`200`	`200`	`use std::borrow::Borrow;`
`201`	`201`	`use std::cell::Cell;`
`202`	`202`