Skip to content

Commit ab3532f

Browse files
Rollup merge of #154581 - GrigorenkoPV:extract-if-debug, r=Mark-Simulacrum
More informative `Debug for vec::ExtractIf` While working on #154318, I've realized that `vec::ExtractIf` actually maintains a valid prefix and a valid suffix in the underlying vector at all times with all the temporarily-invalid elements being only in the middle. So why not make the `Debug` output more informative? I guess we could even expose something like `get_retained_mut(&mut self) -> &mut [T]` and `get_remainder_mut(&mut self) -> &mut [T]`, but that would add new backwards compatibility burden, unlike the `Debug` implementation, which (IIRC) can be changed at any time.
2 parents 69751ec + 6c46776 commit ab3532f

2 files changed

Lines changed: 31 additions & 22 deletions

File tree

library/alloc/src/vec/extract_if.rs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -130,20 +130,25 @@ where
130130
A: Allocator,
131131
{
132132
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
133-
let peek = if self.idx < self.end {
134-
// This has to use pointer arithmetic as `self.vec[self.idx]` or
135-
// `self.vec.get_unchecked(self.idx)` wouldn't work since we
136-
// temporarily set the length of `self.vec` to zero.
137-
//
138-
// SAFETY:
139-
// Since `self.idx` is smaller than `self.end` and `self.end` is
140-
// smaller than `self.old_len`, `idx` is valid for indexing the
141-
// buffer. Also, per the invariant of `self.idx`, this element
142-
// has not been inspected/moved out yet.
143-
Some(unsafe { &*self.vec.as_ptr().add(self.idx) })
144-
} else {
145-
None
146-
};
147-
f.debug_struct("ExtractIf").field("peek", &peek).finish_non_exhaustive()
133+
// We have to use pointer arithmetics here,
134+
// because the length of `self.vec` is temporarily set to 0.
135+
let start = self.vec.as_ptr();
136+
137+
// SAFETY: we always keep first `self.idx - self.del` elements valid.
138+
let retained = unsafe { slice::from_raw_parts(start, self.idx - self.del) };
139+
140+
// SAFETY: we have not yet touched elements starting at `self.idx`.
141+
let valid_tail =
142+
unsafe { slice::from_raw_parts(start.add(self.idx), self.old_len - self.idx) };
143+
144+
// SAFETY: `end - idx <= old_len - idx`, because `end <= old_len`. Also `idx <= end` by invariant.
145+
let (remainder, skipped_tail) =
146+
unsafe { valid_tail.split_at_unchecked(self.end - self.idx) };
147+
148+
f.debug_struct("ExtractIf")
149+
.field("retained", &retained)
150+
.field("remainder", &remainder)
151+
.field("skipped_tail", &skipped_tail)
152+
.finish_non_exhaustive()
148153
}
149154
}

library/alloctests/tests/vec.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,13 +1651,17 @@ fn extract_if_unconsumed() {
16511651

16521652
#[test]
16531653
fn extract_if_debug() {
1654-
let mut vec = vec![1, 2];
1655-
let mut drain = vec.extract_if(.., |&mut x| x % 2 != 0);
1656-
assert!(format!("{drain:?}").contains("Some(1)"));
1657-
drain.next();
1658-
assert!(format!("{drain:?}").contains("Some(2)"));
1659-
drain.next();
1660-
assert!(format!("{drain:?}").contains("None"));
1654+
let mut vec = vec![1, 2, 3, 4, 5, 6, 7, 8];
1655+
let mut drain = vec.extract_if(1..5, |&mut x| x % 2 != 0);
1656+
assert_eq!(
1657+
format!("{drain:?}"),
1658+
"ExtractIf { retained: [1], remainder: [2, 3, 4, 5], skipped_tail: [6, 7, 8], .. }"
1659+
);
1660+
drain.next().unwrap();
1661+
assert_eq!(
1662+
format!("{drain:?}"),
1663+
"ExtractIf { retained: [1, 2], remainder: [4, 5], skipped_tail: [6, 7, 8], .. }"
1664+
);
16611665
}
16621666

16631667
#[test]

0 commit comments

Comments
 (0)