alloc: Added `as_slice` method to `BinaryHeap` collection #82331

frol · 2021-02-20T13:26:09Z

I initially asked about whether it is useful addition on https://internals.rust-lang.org/t/should-i-add-as-slice-method-to-binaryheap/13816, and it seems there were no objections, so went ahead with this PR.

There is BinaryHeap::into_vec, but it consumes the value. I wonder if there is API design limitation that should be taken into account. Implementation-wise, the inner buffer is just a Vec, so it is trivial to expose as_slice from it.

Please, guide me through if I need to add tests or something else.

UPD: Tracking issue #83659

rust-highfive · 2021-02-20T13:26:11Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

frol · 2021-02-25T10:28:54Z

@kennytm I am sending a friendly ping here. Is there something I should improve in this PR?

kennytm

cc @rust-lang/libs.

If no one opposes I'll r+ after the tracking issue is filed.

kennytm · 2021-02-26T03:26:08Z

library/alloc/src/collections/binary_heap.rs

+    ///     println!("{}", x);
+    /// }
+    /// ```
+    #[unstable(feature = "binary_heap_as_slice", issue = "82331")]


the number needs to be a tracking issue, not this PR

dtolnay

Can you share a concrete real world use case for this?

dtolnay · 2021-02-26T04:09:42Z

library/alloc/src/collections/binary_heap.rs

+    /// let heap = BinaryHeap::from(vec![1, 2, 3, 4, 5, 6, 7]);
+    /// let slice = heap.as_slice();
+    ///
+    /// // Will print in some order
+    /// for x in slice {
+    ///     println!("{}", x);
+    /// }


This is going to need a more realistic example. The point of example code is chiefly to show why you would want to call the method, not how to perform a method call. If someone is reading this documentation, we can assume they know how to make method calls. The example here does nothing to illustrate why as_slice would be used, since if you needed to do this, you should be writing for x in &heap {...} instead.

@dtolnay Good question. I had bumped into it when I needed to efficiently serialize BinaryHeap. Some APIs (writer::write_all in my case) may expect a slice of data, and it is going to be quite inefficient to clone the data or iterate over it item by item (when you only have a reference to the BinaryHeap and cannot use into_vec)

m-ou-se · 2021-03-03T18:07:39Z

Related: #82002

frol · 2021-03-13T15:26:21Z

I could not find a more realistic example in the scope of std library, so I took the example from Vec::as_slice. @dtolnay If you feel that it is not worth adding to the standard library, I am fine to close this PR.

JohnCSimon · 2021-03-29T07:46:24Z

Ping from triage:
@frol looks like dtolnay hasn't responded to this... do you want to close this PR?

frol · 2021-03-29T08:40:35Z

@dtolnay I am kindly pinging you to make the decision

Amanieu · 2021-03-29T17:56:52Z

I'm happy with the example as it is right now, so r=me once a tracking issue is filed and the issue number in the PR is updated.

frol · 2021-03-29T19:45:55Z

r? @Amanieu

dtolnay · 2021-03-29T20:14:53Z

@bors r=Amanieu

bors · 2021-03-29T20:14:54Z

📌 Commit 595f3f2 has been approved by Amanieu

Rollup of 7 pull requests Successful merges: - rust-lang#82331 (alloc: Added `as_slice` method to `BinaryHeap` collection) - rust-lang#83130 (escape_ascii take 2) - rust-lang#83374 (unix: Fix feature(unix_socket_ancillary_data) on macos and other BSDs) - rust-lang#83543 (Lint on unknown intra-doc link disambiguators) - rust-lang#83636 (Add a regression test for issue-82792) - rust-lang#83643 (Remove a FIXME resolved by rust-lang#73578) - rust-lang#83644 (:arrow_up: rust-analyzer) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…rntSushi Stabilize `binary_heap_as_slice` This PR stabilizes `binary_heap_as_slice`: ```rust // std::collections::BinaryHeap impl BinaryHeap<T> { pub fn as_slice(&self) -> &[T] } ``` <br> Tracking issue: rust-lang#83659. Implementation PR: rust-lang#82331. FCPs already completed in the tracking issue. Closes rust-lang#83659. r? libs-api

Rollup merge of rust-lang#124012 - slanterns:as_slice_stabilize, r=BurntSushi Stabilize `binary_heap_as_slice` This PR stabilizes `binary_heap_as_slice`: ```rust // std::collections::BinaryHeap impl BinaryHeap<T> { pub fn as_slice(&self) -> &[T] } ``` <br> Tracking issue: rust-lang#83659. Implementation PR: rust-lang#82331. FCPs already completed in the tracking issue. Closes rust-lang#83659. r? libs-api

rust-highfive assigned kennytm Feb 20, 2021

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

This comment has been minimized.

Sign in to view

frol force-pushed the feat/std-binary-heap-as-slice branch from 5dfb4cc to 05e615b Compare February 20, 2021 13:32

This comment has been minimized.

Sign in to view

alloc: Added as_slice method to BinaryHeap collection

6233f3f

frol force-pushed the feat/std-binary-heap-as-slice branch from 05e615b to 6233f3f Compare February 20, 2021 18:46

kennytm reviewed Feb 26, 2021

View reviewed changes

dtolnay reviewed Feb 26, 2021

View reviewed changes

dtolnay requested changes Feb 26, 2021

View reviewed changes

Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 27, 2021

m-ou-se mentioned this pull request Mar 3, 2021

Add BinaryHeap::contains and BinaryHeap::remove #82002

Closed

provide a more realistic example for BinaryHeap::as_slice

dd2b8a0

m-ou-se assigned Amanieu and unassigned kennytm Mar 29, 2021

m-ou-se added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 29, 2021

frol mentioned this pull request Mar 29, 2021

Tracking Issue for alloc::collections::BinaryHeap::as_slice #83659

Closed

3 tasks

Updated the tracking issue #

595f3f2

bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 29, 2021

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 29, 2021

Dylan-DPC-zz mentioned this pull request Mar 29, 2021

Rollup of 7 pull requests #83664

Merged

bors merged commit 2843baa into rust-lang:master Mar 30, 2021

rustbot added this to the 1.53.0 milestone Mar 30, 2021

dtolnay self-assigned this Mar 24, 2024

slanterns mentioned this pull request Apr 16, 2024

Stabilize binary_heap_as_slice #124012

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

alloc: Added `as_slice` method to `BinaryHeap` collection #82331

alloc: Added `as_slice` method to `BinaryHeap` collection #82331

frol commented Feb 20, 2021 •

edited

Loading

rust-highfive commented Feb 20, 2021

This comment has been minimized.

This comment has been minimized.

frol commented Feb 25, 2021

kennytm left a comment

kennytm Feb 26, 2021

dtolnay left a comment

dtolnay Feb 26, 2021

frol Mar 1, 2021 •

edited

Loading

m-ou-se commented Mar 3, 2021

frol commented Mar 13, 2021

JohnCSimon commented Mar 29, 2021

frol commented Mar 29, 2021

Amanieu commented Mar 29, 2021

frol commented Mar 29, 2021

dtolnay commented Mar 29, 2021

bors commented Mar 29, 2021

alloc: Added as_slice method to BinaryHeap collection #82331

alloc: Added as_slice method to BinaryHeap collection #82331

Conversation

frol commented Feb 20, 2021 • edited Loading

rust-highfive commented Feb 20, 2021

This comment has been minimized.

This comment has been minimized.

frol commented Feb 25, 2021

kennytm left a comment

Choose a reason for hiding this comment

kennytm Feb 26, 2021

Choose a reason for hiding this comment

dtolnay left a comment

Choose a reason for hiding this comment

dtolnay Feb 26, 2021

Choose a reason for hiding this comment

frol Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

m-ou-se commented Mar 3, 2021

frol commented Mar 13, 2021

JohnCSimon commented Mar 29, 2021

frol commented Mar 29, 2021

Amanieu commented Mar 29, 2021

frol commented Mar 29, 2021

dtolnay commented Mar 29, 2021

bors commented Mar 29, 2021

alloc: Added `as_slice` method to `BinaryHeap` collection #82331

alloc: Added `as_slice` method to `BinaryHeap` collection #82331

frol commented Feb 20, 2021 •

edited

Loading

frol Mar 1, 2021 •

edited

Loading