Update VecDeque implementation to use head+len instead of head+tail
(See #99805)
This changes `alloc::collections::VecDeque`'s internal representation from using head and tail indices to using a head index and a length field. It has a few advantages over the current design:
* It allows the buffer to be of length 0, which means the `VecDeque::new` new longer has to allocate and could be changed to a `const fn`
* It allows the `VecDeque` to fill the buffer completely, unlike the old implementation, which always had to leave a free space
* It removes the restriction for the size to be a power of two, allowing it to properly `shrink_to_fit`, unlike the old `VecDeque`
* The above points also combine to allow the `Vec<T> -> VecDeque<T>` conversion to be very cheap and guaranteed O(1). I mention this in the `From<Vec<T>>` impl, but it's not a strong guarantee just yet, as that would likely need some form of API change proposal.
All the tests seem to pass for the new `VecDeque`, with some slight adjustments.
r? `@scottmcm`
`VecDeque::resize` should re-use the buffer in the passed-in element
Today it always copies it for *every* appended element, but one of those clones is avoidable.
This adds `iter::repeat_n` (https://github.com/rust-lang/rust/issues/104434) as the primitive needed to do this. If this PR is acceptable, I'll also use this in `Vec` rather than its custom `ExtendElement` type & infrastructure that is harder to share between multiple different containers:
101e1822c3/library/alloc/src/vec/mod.rs (L2479-L2492)
Attempt to reuse `Vec<T>` backing storage for `Rc/Arc<[T]>`
If a `Vec<T>` has sufficient capacity to store the inner `RcBox<[T]>`, we can just reuse the existing allocation and shift the elements up, instead of making a new allocation.
Currently pretty much all of the btree_map and btree_set ones fail, as
well as linked_list::DrainFilter.
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:38:5
|
38 | / require_send_sync(async {
39 | | let _v = None::<alloc::collections::btree_map::Iter<'_, &u32, &u32>>;
40 | | async {}.await;
41 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:56:5
|
56 | / require_send_sync(async {
57 | | let _v = None::<
58 | | alloc::collections::btree_map::DrainFilter<
59 | | '_,
... |
65 | | async {}.await;
66 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:68:5
|
68 | / require_send_sync(async {
69 | | let _v = None::<alloc::collections::btree_map::Entry<'_, &u32, &u32>>;
70 | | async {}.await;
71 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:88:5
|
88 | / require_send_sync(async {
89 | | let _v = None::<alloc::collections::btree_map::Iter<'_, &u32, &u32>>;
90 | | async {}.await;
91 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:93:5
|
93 | / require_send_sync(async {
94 | | let _v = None::<alloc::collections::btree_map::IterMut<'_, &u32, &u32>>;
95 | | async {}.await;
96 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:98:5
|
98 | / require_send_sync(async {
99 | | let _v = None::<alloc::collections::btree_map::Keys<'_, &u32, &u32>>;
100 | | async {}.await;
101 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:103:5
|
103 | / require_send_sync(async {
104 | | let _v = None::<alloc::collections::btree_map::OccupiedEntry<'_, &u32, &u32>>;
105 | | async {}.await;
106 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:108:5
|
108 | / require_send_sync(async {
109 | | let _v = None::<alloc::collections::btree_map::OccupiedError<'_, &u32, &u32>>;
110 | | async {}.await;
111 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:113:5
|
113 | / require_send_sync(async {
114 | | let _v = None::<alloc::collections::btree_map::Range<'_, &u32, &u32>>;
115 | | async {}.await;
116 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:118:5
|
118 | / require_send_sync(async {
119 | | let _v = None::<alloc::collections::btree_map::RangeMut<'_, &u32, &u32>>;
120 | | async {}.await;
121 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:123:5
|
123 | / require_send_sync(async {
124 | | let _v = None::<alloc::collections::btree_map::VacantEntry<'_, &u32, &u32>>;
125 | | async {}.await;
126 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:128:5
|
128 | / require_send_sync(async {
129 | | let _v = None::<alloc::collections::btree_map::Values<'_, &u32, &u32>>;
130 | | async {}.await;
131 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:133:5
|
133 | / require_send_sync(async {
134 | | let _v = None::<alloc::collections::btree_map::ValuesMut<'_, &u32, &u32>>;
135 | | async {}.await;
136 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:146:5
|
146 | / require_send_sync(async {
147 | | let _v = None::<alloc::collections::btree_set::Difference<'_, &u32>>;
148 | | async {}.await;
149 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: implementation of `Send` is not general enough
--> library/alloc/tests/autotraits.rs:151:5
|
151 | / require_send_sync(async {
152 | | let _v = None::<alloc::collections::btree_set::DrainFilter<'_, &u32, fn(&&u32) -> bool>>;
153 | | async {}.await;
154 | | });
| |______^ implementation of `Send` is not general enough
|
= note: `Send` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`...
= note: ...but `Send` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:156:5
|
156 | / require_send_sync(async {
157 | | let _v = None::<alloc::collections::btree_set::Intersection<'_, &u32>>;
158 | | async {}.await;
159 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:166:5
|
166 | / require_send_sync(async {
167 | | let _v = None::<alloc::collections::btree_set::Iter<'_, &u32>>;
168 | | async {}.await;
169 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:171:5
|
171 | / require_send_sync(async {
172 | | let _v = None::<alloc::collections::btree_set::Range<'_, &u32>>;
173 | | async {}.await;
174 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:176:5
|
176 | / require_send_sync(async {
177 | | let _v = None::<alloc::collections::btree_set::SymmetricDifference<'_, &u32>>;
178 | | async {}.await;
179 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: higher-ranked lifetime error
--> library/alloc/tests/autotraits.rs:181:5
|
181 | / require_send_sync(async {
182 | | let _v = None::<alloc::collections::btree_set::Union<'_, &u32>>;
183 | | async {}.await;
184 | | });
| |______^
|
= note: could not prove `impl Future<Output = ()>: Send`
error: future cannot be sent between threads safely
--> library/alloc/tests/autotraits.rs:243:23
|
243 | require_send_sync(async {
| _______________________^
244 | | let _v =
245 | | None::<alloc::collections::linked_list::DrainFilter<'_, &u32, fn(&mut &u32) -> bool>>;
246 | | async {}.await;
247 | | });
| |_____^ future created by async block is not `Send`
|
= help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `NonNull<std::collections::linked_list::Node<&u32>>`
note: future is not `Send` as this value is used across an await
--> library/alloc/tests/autotraits.rs:246:17
|
244 | let _v =
| -- has type `Option<std::collections::linked_list::DrainFilter<'_, &u32, for<'a, 'b> fn(&'a mut &'b u32) -> bool>>` which is not `Send`
245 | None::<alloc::collections::linked_list::DrainFilter<'_, &u32, fn(&mut &u32) -> bool>>;
246 | async {}.await;
| ^^^^^^ await occurs here, with `_v` maybe used later
247 | });
| - `_v` is later dropped here
note: required by a bound in `require_send_sync`
--> library/alloc/tests/autotraits.rs:3:25
|
3 | fn require_send_sync<T: Send + Sync>(_: T) {}
| ^^^^ required by this bound in `require_send_sync`
error: future cannot be shared between threads safely
--> library/alloc/tests/autotraits.rs:243:23
|
243 | require_send_sync(async {
| _______________________^
244 | | let _v =
245 | | None::<alloc::collections::linked_list::DrainFilter<'_, &u32, fn(&mut &u32) -> bool>>;
246 | | async {}.await;
247 | | });
| |_____^ future created by async block is not `Sync`
|
= help: within `impl Future<Output = ()>`, the trait `Sync` is not implemented for `NonNull<std::collections::linked_list::Node<&u32>>`
note: future is not `Sync` as this value is used across an await
--> library/alloc/tests/autotraits.rs:246:17
|
244 | let _v =
| -- has type `Option<std::collections::linked_list::DrainFilter<'_, &u32, for<'a, 'b> fn(&'a mut &'b u32) -> bool>>` which is not `Sync`
245 | None::<alloc::collections::linked_list::DrainFilter<'_, &u32, fn(&mut &u32) -> bool>>;
246 | async {}.await;
| ^^^^^^ await occurs here, with `_v` maybe used later
247 | });
| - `_v` is later dropped here
note: required by a bound in `require_send_sync`
--> library/alloc/tests/autotraits.rs:3:32
|
3 | fn require_send_sync<T: Send + Sync>(_: T) {}
| ^^^^ required by this bound in `require_send_sync`
Fix in-place collection leak when remaining element destructor panic
Fixes#101628
cc `@the8472`
I went for the drop guard route, placing it immediately before the `forget_allocation_drop_remaining` call and after the comment, as to signal they are closely related.
I also updated the test to check for the leak, though the only change really needed was removing the leak clean up for miri since now that's no longer leaked.
Stabilize bench_black_box
This PR stabilize `feature(bench_black_box)`.
```rust
pub fn black_box<T>(dummy: T) -> T;
```
The FCP was completed in https://github.com/rust-lang/rust/issues/64102.
`@rustbot` label +T-libs-api -T-libs
Since `len` and `is_empty` are not const stable yet, this also
creates a new feature for them since they previously used the same
`const_btree_new` feature.
Add `vec::Drain{,Filter}::keep_rest`
This PR adds `keep_rest` methods to `vec::Drain` and `vec::DrainFilter` under `drain_keep_rest` feature gate:
```rust
// mod alloc::vec
impl<T, A: Allocator> Drain<'_, T, A> {
pub fn keep_rest(self);
}
impl<T, F, A: Allocator> DrainFilter<'_, T, F, A>
where
F: FnMut(&mut T) -> bool,
{
pub fn keep_rest(self);
}
```
Both these methods cancel draining of elements that were not yet yielded from the iterators. While this needs more testing & documentation, I want at least start the discussion. This may be a potential way out of the "should `DrainFilter` exhaust itself on drop?" argument.
Use pointer `is_aligned*` methods
This PR replaces some manual alignment checks with calls to `pointer::{is_aligned, is_aligned_to}` and removes a useless pointer cast.
r? `@scottmcm`
_split off from #100746_
Add tests that check `Vec::retain` predicate execution order.
This behaviour is documented for `Vec::retain` which means that there is code that rely on that but there weren't tests about that.
Optimized vec::IntoIter::next_chunk impl
```
x86_64v1, default
test vec::bench_next_chunk ... bench: 696 ns/iter (+/- 22)
x86_64v1, pr
test vec::bench_next_chunk ... bench: 309 ns/iter (+/- 4)
znver2, default
test vec::bench_next_chunk ... bench: 17,272 ns/iter (+/- 117)
znver2, pr
test vec::bench_next_chunk ... bench: 211 ns/iter (+/- 3)
```
On znver2 the default impl seems to be slow due to different inlining decisions. It goes through `core::array::iter_next_chunk`
which has a deep call tree.
Stabilize the `core_c_str` and `alloc_c_string` feature gates.
Change `std::ffi` to re-export these types rather than creating type
aliases, since they now have matching stability.
Stabilize `core::ffi:c_*` and rexport in `std::ffi`
This only stabilizes the base types, not the non-zero variants, since
those have their own separate tracking issue and have not gone through
FCP to stabilize.
This only stabilizes the base types, not the non-zero variants, since
those have their own separate tracking issue and have not gone through
FCP to stabilize.
Enforce that layout size fits in isize in Layout
As it turns out, enforcing this _in APIs that already enforce `usize` overflow_ is fairly trivial. `Layout::from_size_align_unchecked` continues to "allow" sizes which (when rounded up) would overflow `isize`, but these are now declared as library UB for `Layout`, meaning that consumers of `Layout` no longer have to check this before making an allocation.
(Note that this is "immediate library UB;" IOW it is valid for a future release to make this immediate "language UB," and there is an extant patch to do so, to allow Miri to catch this misuse.)
See also #95252, [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Layout.20Isn't.20Enforcing.20The.20isize.3A.3AMAX.20Rule).
Fixes https://github.com/rust-lang/rust/issues/95334
Some relevant quotes:
`@eddyb,` https://github.com/rust-lang/rust/pull/95252#issuecomment-1078513769
> [B]ecause of the non-trivial presence of both of these among code published on e.g. crates.io:
>
> 1. **`Layout` "producers" / `GlobalAlloc` "users"**: smart pointers (including `alloc::rc` copies with small tweaks), collections, etc.
> 2. **`Layout` "consumers" / `GlobalAlloc` "providers"**: perhaps fewer of these, but anything built on top of OS APIs like `mmap` will expose `> isize::MAX` allocations (on 32-bit hosts) if they lack extra checks
>
> IMO the only responsible option is to enforce the `isize::MAX` limit in `Layout`, which:
>
> * makes `Layout` _sound_ in terms of only ever allowing allocations where `(alloc_base_ptr: *mut u8).offset(size)` is never UB
> * frees both "producers" and "consumers" of `Layout` from manually reimplementing the checks
> * manual checks can be risky, e.g. if the final size passed to the allocator isn't the one being checked
> * this applies retroactively, fixing the overall soundness of existing code with zero transition period or _any_ changes required from users (as long as going through `Layout` is mandatory, making a "choke point")
>
>
> Feel free to quote this comment onto any relevant issue, I might not be able to keep track of developments.
`@Gankra,` https://github.com/rust-lang/rust/pull/95252#issuecomment-1078556371
> As someone who spent way too much time optimizing libcollections checks for this stuff and tried to splatter docs about it everywhere on the belief that it was a reasonable thing for people to manually take care of: I concede the point, it is not reasonable. I am wholy spiritually defeated by the fact that _liballoc_ of all places is getting this stuff wrong. This isn't throwing shade at the folks who implemented these Rc features, but rather a statement of how impractical it is to expect anyone out in the wider ecosystem to enforce them if _some of the most audited rust code in the library that defines the very notion of allocating memory_ can't even reliably do it.
>
> We need the nuclear option of Layout enforcing this rule. Code that breaks this rule is _deeply_ broken and any "regressions" from changing Layout's contract is a _correctness_ fix. Anyone who disagrees and is sufficiently motivated can go around our backs but the standard library should 100% refuse to enable them.
cc also `@RalfJung` `@rust-lang/wg-allocators.` Even though this technically supersedes #95252, those potential failure points should almost certainly still get nicer panics than just "unwrap failed" (which they would get by this PR).
It might additionally be worth recommending to users of the `Layout` API that they should ideally use `.and_then`/`?` to complete the entire layout calculation, and then `panic!` from a single location at the end of `Layout` manipulation, to reduce the overhead of the checks and optimizations preserving the exact location of each `panic` which are conceptually just one failure: allocation too big.
Probably deserves a T-lang and/or T-libs-api FCP (this technically solidifies the [objects must be no larger than `isize::MAX`](https://rust-lang.github.io/unsafe-code-guidelines/layout/scalars.html#isize-and-usize) rule further, and the UCG document says this hasn't been RFCd) and a crater run. Ideally, no code exists that will start failing with this addition; if it does, it was _likely_ (but not certainly) causing UB.
Changes the raw_vec allocation path, thus deserves a perf run as well.
I suggest hiding whitespace-only changes in the diff view.
Make `ThinBox<T>` covariant in `T`
Just like `Box<T>`, we want `ThinBox<T>` to be covariant in `T`, but the
projection in `WithHeader<<T as Pointee>::Metadata>` was making it
invariant. This is now hidden as `WithOpaqueHeader`, which we type-cast
whenever the real `WithHeader<H>` type is needed.
Fixes the problem noted in <https://github.com/rust-lang/rust/issues/92791#issuecomment-1104636249>.
Just like `Box<T>`, we want `ThinBox<T>` to be covariant in `T`, but the
projection in `WithHeader<<T as Pointee>::Metadata>` was making it
invariant. This is now hidden as `WithOpaqueHeader`, which we type-cast
whenever the real `WithHeader<H>` type is needed.
Documentation for the following methods
with_capacity
with_capacity_in
with_capacity_and_hasher
reserve
reserve_exact
try_reserve
try_reserve_exact
was inconsistent and often not entirely correct where they existed on the following types
Vec
VecDeque
String
OsString
PathBuf
BinaryHeap
HashSet
HashMap
BufWriter
LineWriter
since the allocator is allowed to allocate more than the requested capacity in all such cases, and will frequently "allocate" much more in the case of zero-sized types (I also checked BufReader, but there the docs appear to be accurate as it appears to actually allocate the exact capacity).
Some effort was made to make the documentation more consistent between types as well.
Fix with_capacity* methods for Vec
Fix *reserve* methods for Vec
Fix docs for *reserve* methods of VecDeque
Fix docs for String::with_capacity
Fix docs for *reserve* methods of String
Fix docs for OsString::with_capacity
Fix docs for *reserve* methods on OsString
Fix docs for with_capacity* methods on HashSet
Fix docs for *reserve methods of HashSet
Fix docs for with_capacity* methods of HashMap
Fix docs for *reserve methods on HashMap
Fix expect messages about OOM in doctests
Fix docs for BinaryHeap::with_capacity
Fix docs for *reserve* methods of BinaryHeap
Fix typos
Fix docs for with_capacity on BufWriter and LineWriter
Fix consistent use of `hasher` between `HashMap` and `HashSet`
Fix warning in doc test
Add test for capacity of vec with ZST
Fix doc test error
Avoid zero-sized allocs in ThinBox if T and H are both ZSTs.
This was surprisingly tricky, and took longer to get right than expected. `ThinBox` is a surprisingly subtle piece of code. That said, in the end, a lot of this was due to overthinking[^overthink] -- ultimately the fix ended up fairly clean and simple.
[^overthink]: Honestly, for a while I was convinced this couldn't be done without allocations or runtime branches in these cases, but that's obviously untrue.
Anyway, as a result of spending all that time debugging, I've extended the tests quite a bit, and also added more debug assertions. Many of these helped for subtle bugs I made in the middle (for example, the alloc/drop tracking is because I ended up double-dropping the value in the case where both were ZSTs), they're arguably a bit of overkill at this point, although I imagine they could help in the future too.
Anyway, these tests cover a wide range of size/align cases, nd fully pass under miri[^1]. They also do some smoke-check asserting that the value has the correct alignment, although in practice it's totally within the compiler's rights to delete these assertions since we'd have already done UB if they get hit. They have more boilerplate than they really need, but it's not *too* bad on a per-test basis.
A notable absence from testing is atypical header types, but at the moment it's impossible to manually implement `Pointee`. It would be really nice to have testing here, since it's not 100% obvious to me that the aligned read/write we use for `H` are correct in the face of arbitrary combinations of `size_of::<H>()`, `align_of::<H>()`, and `align_of::<T>()`. (That said, I spent a while thinking through it and am *pretty* sure it's fine -- I'd just feel... better if we could test some cases for non-ZST headers which have unequal and align).
[^1]: Or at least, they pass under miri if I copy the code and tests into a new crate and run miri on it (after making it less stdlibified).
Fixes#96485.
I'd request review ``@yaahc,`` but I believe you're taking some time away from reviews, so I'll request from the previous PR's reviewer (I think that the context helps, even if the actual change didn't end up being bad here).
r? ``@joshtriplett``
