Add support for custom allocator in `VecDeque` #86595

a1phyr · 2021-06-24T12:52:35Z

This follows the roadmap of the allocator WG to add custom allocators to collections.

@rustbot modify labels: +A-allocators +T-libs

rust-highfive · 2021-06-24T12:52:38Z

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

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

jyn514 · 2021-07-08T12:12:38Z

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

-__impl_slice_eq1! { [const N: usize] VecDeque<A>, [B; N], }
-__impl_slice_eq1! { [const N: usize] VecDeque<A>, &[B; N], }
-__impl_slice_eq1! { [const N: usize] VecDeque<A>, &mut [B; N], }
+__impl_slice_eq1! { [] VecDeque<T, A>, Vec<U, A>, }


Hmm, why do you require both to come from the same allocator? Just comparing them shouldn't do any reallocation, right?

For all function potentially mixing allocators, while I agree that allowing it would be great, current APIs (eg for Vec) do not seem to go in this direction.

There may be a reason I don't know for this choice, but I chose consistency anyway. If the opposite is decided, I'll be happy to change this.

jyn514 · 2021-07-08T12:13:13Z

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

-impl<A: PartialEq> PartialEq for VecDeque<A> {
-    fn eq(&self, other: &VecDeque<A>) -> bool {
+impl<T: PartialEq, A: Allocator> PartialEq for VecDeque<T, A> {
+    fn eq(&self, other: &Self) -> bool {


Ditto: why require the same allocator? Is it required by the trait definition?

This is not required by the definition (only for Eq), but this is what Vec does, so I did the same.

jyn514 · 2021-07-08T12:15:08Z

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

        self.get_mut(index).expect("Out of bounds access")
    }
 }

 #[stable(feature = "rust1", since = "1.0.0")]
-impl<A> FromIterator<A> for VecDeque<A> {
-    fn from_iter<T: IntoIterator<Item = A>>(iter: T) -> VecDeque<A> {
+impl<T> FromIterator<T> for VecDeque<T> {


I think you forgot to add an A: Allocator parameter here.

I was not sure about this one, as adding a generic parameter could break inference. So I referred to what Vec does and did the same.

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

jyn514 · 2021-07-08T12:19:18Z

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

    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        f.debug_list().entries(self).finish()
    }
 }

 #[stable(feature = "vecdeque_vec_conversions", since = "1.10.0")]
-impl<T> From<Vec<T>> for VecDeque<T> {
+impl<T, A: Allocator> From<Vec<T, A>> for VecDeque<T, A> {


Ditto: why require the same allocator?

jyn514 · 2021-07-08T12:20:07Z

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

            VecDeque { tail: 0, head: len, buf }
        }
    }
 }

 #[stable(feature = "vecdeque_vec_conversions", since = "1.10.0")]
-impl<T> From<VecDeque<T>> for Vec<T> {
+impl<T, A: Allocator> From<VecDeque<T, A>> for Vec<T, A> {


Ditto: why require the same allocator?

jyn514 · 2021-07-08T12:20:34Z

library/alloc/src/collections/vec_deque/pair_slices.rs

@@ -25,7 +27,7 @@ pub struct PairSlices<'a, 'b, T> {
 }

 impl<'a, 'b, T> PairSlices<'a, 'b, T> {
-    pub fn from(to: &'a mut VecDeque<T>, from: &'b VecDeque<T>) -> Self {
+    pub fn from<A: Allocator>(to: &'a mut VecDeque<T, A>, from: &'b VecDeque<T, A>) -> Self {


Ditto: why require the same allocator?

bors · 2021-07-22T15:29:26Z

☔ The latest upstream changes (presumably #87366) made this pull request unmergeable. Please resolve the merge conflicts.

jyn514 · 2021-07-22T16:16:41Z

r? @Amanieu

Amanieu · 2021-07-24T23:32:48Z

I'm happy with the changes since it mirrors what Vec does.

@bors r+

bors · 2021-07-24T23:32:50Z

📌 Commit 19318e6 has been approved by Amanieu

bors · 2021-07-25T01:12:42Z

⌛ Testing commit 19318e6 with merge ca1bb41863911446e5f687c523052113c7716d9e...

bors · 2021-07-25T01:47:39Z

💔 Test failed - checks-actions

Amanieu · 2021-07-25T18:35:14Z

@bors r+

bors · 2021-07-25T18:35:15Z

📌 Commit 8987b74 has been approved by Amanieu

bors · 2021-07-25T19:01:15Z

⌛ Testing commit 8987b74 with merge 9c25eb7...

bors · 2021-07-25T21:41:52Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 9c25eb7 to master...

rust-highfive assigned kennytm Jun 24, 2021

rustbot added A-allocators Area: Custom and system allocators T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 24, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2021

TimDiekmann mentioned this pull request Jun 24, 2021

Tracking Issue for structs which needs an allocator rust-lang/wg-allocators#7

Open

15 tasks

jyn514 suggested changes Jul 8, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

a1phyr force-pushed the allocator_api_for_vecdeque branch from 4b52881 to 821b4b8 Compare July 8, 2021 14:34

rust-highfive assigned Amanieu and unassigned kennytm Jul 22, 2021

a1phyr added 3 commits July 23, 2021 20:37

Add support for custom allocator in VecDeque

0570f09

Add unstable attribute for A in Drain and IntoIter

6fcc62b

Add #[unstable] on new functions

19318e6

a1phyr force-pushed the allocator_api_for_vecdeque branch from 821b4b8 to 19318e6 Compare July 23, 2021 18:38

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2021

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 25, 2021

This comment has been minimized.

Sign in to view

Fix failing test

8987b74

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 25, 2021

bors merged commit 9c25eb7 into rust-lang:master Jul 25, 2021

rustbot added this to the 1.56.0 milestone Jul 25, 2021

bors mentioned this pull request Jul 25, 2021

Remove unsound TrustedRandomAccess implementations #85874

Merged

a1phyr deleted the allocator_api_for_vecdeque branch March 12, 2024 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for custom allocator in `VecDeque` #86595

Add support for custom allocator in `VecDeque` #86595

a1phyr commented Jun 24, 2021

rust-highfive commented Jun 24, 2021

jyn514 Jul 8, 2021

a1phyr Jul 8, 2021

jyn514 Jul 8, 2021

a1phyr Jul 8, 2021

jyn514 Jul 8, 2021

a1phyr Jul 8, 2021

jyn514 Jul 8, 2021

jyn514 Jul 8, 2021

jyn514 Jul 8, 2021

This comment has been minimized.

bors commented Jul 22, 2021

jyn514 commented Jul 22, 2021

Amanieu commented Jul 24, 2021

bors commented Jul 24, 2021

bors commented Jul 25, 2021

bors commented Jul 25, 2021

This comment has been minimized.

Amanieu commented Jul 25, 2021

bors commented Jul 25, 2021

bors commented Jul 25, 2021

bors commented Jul 25, 2021

Add support for custom allocator in VecDeque #86595

Add support for custom allocator in VecDeque #86595

Conversation

a1phyr commented Jun 24, 2021

rust-highfive commented Jun 24, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

bors commented Jul 22, 2021

jyn514 commented Jul 22, 2021

Amanieu commented Jul 24, 2021

bors commented Jul 24, 2021

bors commented Jul 25, 2021

bors commented Jul 25, 2021

This comment has been minimized.

Amanieu commented Jul 25, 2021

bors commented Jul 25, 2021

bors commented Jul 25, 2021

bors commented Jul 25, 2021

Add support for custom allocator in `VecDeque` #86595

Add support for custom allocator in `VecDeque` #86595