Skip to content

Commit aa9f27e

Browse files
KobzolKobzol
committed
Rename WrappedIndex::new to from_arbitrary_number and resolve review remarks
1 parent 207d3bd commit aa9f27e

3 files changed

Lines changed: 48 additions & 47 deletions

File tree

‎library/alloc/src/collections/vec_deque/mod.rs‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
510510
self.copy(src, dst, src_pre_wrap_len);
511511
self.copy(WrappedIndex::zero(), dst.add(src_pre_wrap_len), delta);
512512
self.copy(
513-
WrappedIndex::new(delta),
513+
WrappedIndex::from_arbitrary_number(delta),
514514
WrappedIndex::zero(),
515515
len - dst_pre_wrap_len,
516516
);
@@ -531,11 +531,11 @@ impl<T, A: Allocator> VecDeque<T, A> {
531531
unsafe {
532532
self.copy(
533533
WrappedIndex::zero(),
534-
WrappedIndex::new(delta),
534+
WrappedIndex::from_arbitrary_number(delta),
535535
len - src_pre_wrap_len,
536536
);
537537
self.copy(
538-
WrappedIndex::new(self.capacity() - delta),
538+
WrappedIndex::from_arbitrary_number(self.capacity() - delta),
539539
WrappedIndex::zero(),
540540
delta,
541541
);
@@ -702,13 +702,13 @@ impl<T, A: Allocator> VecDeque<T, A> {
702702
unsafe {
703703
self.copy_nonoverlapping(
704704
WrappedIndex::zero(),
705-
WrappedIndex::new(old_capacity),
705+
WrappedIndex::from_arbitrary_number(old_capacity),
706706
tail_len,
707707
);
708708
}
709709
} else {
710710
// C
711-
let new_head = unsafe { WrappedIndex::new(new_capacity - head_len) };
711+
let new_head = WrappedIndex::from_arbitrary_number(new_capacity - head_len);
712712
unsafe {
713713
// can't use copy_nonoverlapping here, because if e.g. head_len = 2
714714
// and new_capacity = old_capacity + 1, then the heads overlap.
@@ -941,7 +941,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
941941
// and that the allocation is valid for use in `RawVec`.
942942
unsafe {
943943
VecDeque {
944-
head: WrappedIndex::new(initialized.start),
944+
head: WrappedIndex::from_arbitrary_number(initialized.start),
945945
len: initialized.end.unchecked_sub(initialized.start),
946946
buf: RawVec::from_raw_parts_in(ptr, capacity, alloc),
947947
}
@@ -1313,7 +1313,11 @@ impl<T, A: Allocator> VecDeque<T, A> {
13131313
let len = self.head + self.len - target_cap;
13141314
// Safety: head is < target_cap, so the index is wrapped
13151315
unsafe {
1316-
self.copy_nonoverlapping(WrappedIndex::new(target_cap), WrappedIndex::zero(), len);
1316+
self.copy_nonoverlapping(
1317+
WrappedIndex::from_arbitrary_number(target_cap),
1318+
WrappedIndex::zero(),
1319+
len,
1320+
);
13171321
}
13181322
} else if !self.is_contiguous() {
13191323
// The head slice is at least partially out of bounds, tail is in bounds.
@@ -1328,8 +1332,8 @@ impl<T, A: Allocator> VecDeque<T, A> {
13281332
// [o o o o o . o o ]
13291333
let head_len = self.capacity() - self.head.as_index();
13301334

1331-
// Safety: head_len is at least one, so new_head will be < target_cap
1332-
let new_head = unsafe { WrappedIndex::new(target_cap - head_len) };
1335+
// head_len is at least one, so new_head will be < target_cap
1336+
let new_head = WrappedIndex::from_arbitrary_number(target_cap - head_len);
13331337
unsafe {
13341338
// can't use `copy_nonoverlapping()` here because the new and old
13351339
// regions for the head might overlap.
@@ -1394,7 +1398,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
13941398
// head slice ends at `target_cap`, so that's where we copy to.
13951399
self.copy_nonoverlapping(
13961400
WrappedIndex::zero(),
1397-
WrappedIndex::new(target_cap),
1401+
WrappedIndex::from_arbitrary_number(target_cap),
13981402
tail_len,
13991403
);
14001404
}
@@ -2909,8 +2913,10 @@ impl<T, A: Allocator> VecDeque<T, A> {
29092913
let free = cap - len;
29102914
let head_len = cap - head.as_index();
29112915

2912-
// Safety: tail <= head_len <= len <= capacity
2913-
let tail = unsafe { WrappedIndex::new(len - head_len) };
2916+
// tail <= head < capacity
2917+
// head cannot be <= capacity, because we know that VecDeque is non-empty, since it is not
2918+
// contiguous at this point
2919+
let tail = WrappedIndex::from_arbitrary_number(len - head_len);
29142920
let tail_len = tail.as_index();
29152921

29162922
if free >= head_len {
@@ -2921,7 +2927,11 @@ impl<T, A: Allocator> VecDeque<T, A> {
29212927
// from: DEFGH....ABC
29222928
// to: ABCDEFGH....
29232929
unsafe {
2924-
self.copy(WrappedIndex::zero(), WrappedIndex::new(head_len), tail_len);
2930+
self.copy(
2931+
WrappedIndex::zero(),
2932+
WrappedIndex::from_arbitrary_number(head_len),
2933+
tail_len,
2934+
);
29252935
// ...DEFGH.ABC
29262936
self.copy_nonoverlapping(head, WrappedIndex::zero(), head_len);
29272937
// ABCDEFGH....
@@ -2974,7 +2984,11 @@ impl<T, A: Allocator> VecDeque<T, A> {
29742984
// because we only move the tail forward as much as there's free space
29752985
// behind it, we don't overwrite any elements of the head slice, and
29762986
// the slices end up right next to each other.
2977-
self.copy(WrappedIndex::zero(), WrappedIndex::new(free), tail_len);
2987+
self.copy(
2988+
WrappedIndex::zero(),
2989+
WrappedIndex::from_arbitrary_number(free),
2990+
tail_len,
2991+
);
29782992
}
29792993

29802994
// We just copied the tail right next to the head slice,
@@ -2987,7 +3001,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
29873001

29883002
// the used part of the buffer now is `free..self.capacity()`, so set
29893003
// `head` to the beginning of that range.
2990-
self.head = WrappedIndex::new(free);
3004+
self.head = WrappedIndex::from_arbitrary_number(free);
29913005
}
29923006
} else {
29933007
// head is shorter so:
@@ -3000,7 +3014,11 @@ impl<T, A: Allocator> VecDeque<T, A> {
30003014
// right next to each other and we don't need to move any memory.
30013015
if free != 0 {
30023016
// copy the head slice to lie right behind the tail slice.
3003-
self.copy(self.head, WrappedIndex::new(tail_len), head_len);
3017+
self.copy(
3018+
self.head,
3019+
WrappedIndex::from_arbitrary_number(tail_len),
3020+
head_len,
3021+
);
30043022
}
30053023

30063024
// because we copied the head slice so that both slices lie right
@@ -3541,7 +3559,7 @@ impl<T: Clone, A: Allocator> SpecExtendFromWithin for VecDeque<T, A> {
35413559
if self.head.is_zero() {
35423560
// SAFETY: the wrapped index may be temporarily equal to the capacity even if it
35433561
// is not zero, because we subtract it one line below.
3544-
self.head = WrappedIndex::new(cap);
3562+
self.head = WrappedIndex::from_arbitrary_number(cap);
35453563
}
35463564
self.head = self.head.sub(1);
35473565
self.len += 1;
@@ -3636,9 +3654,10 @@ mod index {
36363654
pub(super) struct WrappedIndex(usize);
36373655

36383656
impl WrappedIndex {
3639-
/// Safety invariant: the newly constructed index must be in-bounds for the VecDeque
3657+
/// The newly constructed index has to be in-bounds for the VecDeque
3658+
/// that uses the index.
36403659
#[inline(always)]
3641-
pub(super) unsafe fn new(index: usize) -> Self {
3660+
pub(super) fn from_arbitrary_number(index: usize) -> Self {
36423661
Self(index)
36433662
}
36443663

‎library/alloc/src/collections/vec_deque/tests.rs‎

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,7 @@ fn test_swap_front_back_remove() {
109109
let expected: VecDeque<_> =
110110
if back { (0..len).collect() } else { (0..len).rev().collect() };
111111
for head_pos in 0..usable_cap {
112-
unsafe {
113-
tester.head = WrappedIndex::new(head_pos);
114-
}
112+
tester.head = WrappedIndex::from_arbitrary_number(head_pos);
115113
tester.len = 0;
116114
if back {
117115
for i in 0..len * 2 {
@@ -157,9 +155,7 @@ fn test_insert() {
157155
let expected = (0..).take(len).collect::<VecDeque<_>>();
158156
for head_pos in 0..cap {
159157
for to_insert in 0..len {
160-
unsafe {
161-
tester.head = WrappedIndex::new(head_pos);
162-
}
158+
tester.head = WrappedIndex::from_arbitrary_number(head_pos);
163159
tester.len = 0;
164160
for i in 0..len {
165161
if i != to_insert {
@@ -640,9 +636,7 @@ fn test_remove() {
640636
let expected = (0..).take(len).collect::<VecDeque<_>>();
641637
for head_pos in 0..cap {
642638
for to_remove in 0..=len {
643-
unsafe {
644-
tester.head = WrappedIndex::new(head_pos);
645-
}
639+
tester.head = WrappedIndex::from_arbitrary_number(head_pos);
646640
tester.len = 0;
647641
for i in 0..len {
648642
if i == to_remove {
@@ -672,9 +666,7 @@ fn test_range() {
672666
for head in 0..=cap {
673667
for start in 0..=len {
674668
for end in start..=len {
675-
unsafe {
676-
tester.head = WrappedIndex::new(head);
677-
}
669+
tester.head = WrappedIndex::from_arbitrary_number(head);
678670
tester.len = 0;
679671
for i in 0..len {
680672
tester.push_back(i);
@@ -699,9 +691,7 @@ fn test_range_mut() {
699691
for head in 0..=cap {
700692
for start in 0..=len {
701693
for end in start..=len {
702-
unsafe {
703-
tester.head = WrappedIndex::new(head);
704-
}
694+
tester.head = WrappedIndex::from_arbitrary_number(head);
705695
tester.len = 0;
706696
for i in 0..len {
707697
tester.push_back(i);
@@ -735,9 +725,7 @@ fn test_drain() {
735725
for head in 0..cap {
736726
for drain_start in 0..=len {
737727
for drain_end in drain_start..=len {
738-
unsafe {
739-
tester.head = WrappedIndex::new(head);
740-
}
728+
tester.head = WrappedIndex::from_arbitrary_number(head);
741729
tester.len = 0;
742730
for i in 0..len {
743731
tester.push_back(i);
@@ -794,9 +782,7 @@ fn test_shrink_to() {
794782
assert_eq!(deque.capacity(), cap);
795783

796784
// we can let the head point anywhere in the buffer since the deque is empty.
797-
unsafe {
798-
deque.head = WrappedIndex::new(head);
799-
}
785+
deque.head = WrappedIndex::from_arbitrary_number(head);
800786
deque.extend(1..=len);
801787

802788
deque.shrink_to(target_cap);
@@ -825,9 +811,7 @@ fn test_shrink_to_fit() {
825811
let expected = (0..).take(len).collect::<VecDeque<_>>();
826812
for head_pos in 0..=max_cap {
827813
tester.reserve(head_pos);
828-
unsafe {
829-
tester.head = WrappedIndex::new(head_pos);
830-
}
814+
tester.head = WrappedIndex::from_arbitrary_number(head_pos);
831815
tester.len = 0;
832816
tester.reserve(63);
833817
for i in 0..len {
@@ -864,9 +848,7 @@ fn test_split_off() {
864848
let expected_other = (at..).take(len - at).collect::<VecDeque<_>>();
865849

866850
for head_pos in 0..cap {
867-
unsafe {
868-
tester.head = WrappedIndex::new(head_pos);
869-
}
851+
tester.head = WrappedIndex::from_arbitrary_number(head_pos);
870852
tester.len = 0;
871853
for i in 0..len {
872854
tester.push_back(i);

‎src/etc/lldb_providers.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ def StdSliceSummaryProvider(valobj, dict):
10321032
class StdVecDequeSyntheticProvider:
10331033
"""Pretty-printer for alloc::collections::vec_deque::VecDeque<T>
10341034
1035-
struct VecDeque<T> { head: BufferIndex, len: usize, buf: RawVec<T> }
1035+
struct VecDeque<T> { head: WrappedIndex, len: usize, buf: RawVec<T> }
10361036
"""
10371037

10381038
def __init__(self, valobj: SBValue, _dict: LLDBOpaque):

0 commit comments

Comments
 (0)