Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 3 tasks
frol opened this issue Mar 29, 2021 · 9 comments · May be fixed by #124012
Open
1 of 3 tasks

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

frol opened this issue Mar 29, 2021 · 9 comments · May be fixed by #124012
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting

Comments

@frol
Copy link
Contributor

frol commented Mar 29, 2021

Feature gate: #![feature(binary_heap_as_slice)]

This is a tracking issue for std::collections::BinaryHeap::as_slice, a method that returns a slice of the data that is stored in the BinaryHeap (the order is arbitrary).

Public API

impl BinaryHeap<T> {
    pub fn as_slice(&self) -> &[T]
}

Steps / History

Unresolved Questions

  • None yet.
@frol frol added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 29, 2021
@frol frol changed the title Tracking Issue for std::collections::BinaryHeap::as_slice Tracking Issue for alloc::collections::BinaryHeap::as_slice Mar 29, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 29, 2021
… r=Amanieu

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

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`](https://doc.rust-lang.org/std/collections/struct.BinaryHeap.html#method.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 rust-lang#83659
@saolof
Copy link

saolof commented Mar 3, 2023

If this is added, it means that the order of the underlying slice is added to the API, which is useful for some purposes. In that case, is it possible to add a .sort_slice() method that just calls .sort() on the underlying Vec, for when you want that slice to be sorted?

This does mean that the implementation details as a binary heap leaks a bit, but there are cases where you do want to alternate between having a sorted array and just having a binary heap.

@dspyz-matician
Copy link

This seems useful for efficiently selecting a uniform random element.

@SmnTin
Copy link

SmnTin commented Aug 27, 2023

Can we stabilize it?

@dtolnay
Copy link
Member

dtolnay commented Apr 6, 2024

@SmnTin would you be able to share a link or an explanation of a real-world use case?

@dspyz-matician is this a real use case? In what scenario would you be keeping elements in a BinaryHeap when needing to randomly select among them?

@dspyz-matician
Copy link

dspyz-matician commented Apr 6, 2024

@dtolnay I have a RRT-like stochastic anytime search algorithm that continually keeps the top N strategies it's found. It randomly selects one of these strategies, clones it, performs a random modification to get a new strategy, then if the new strategy is better than the current worst, it inserts it and pops the current worst one off the top.

@dtolnay
Copy link
Member

dtolnay commented Apr 6, 2024

Thanks!

@rust-lang/libs-api:
@rfcbot fcp merge

BinaryHeap::as_slice returns &[T] containing the heap elements in arbitrary order. Regarding whether we should be concerned that exposing the elements in some unspecified order will be problematic, there is already a stable into_vec that returns the heap elements as Vec<T>, also documented as returning them in arbitrary order. (There is also into_sorted_vec, which does a heapsort.)

@rfcbot
Copy link

rfcbot commented Apr 6, 2024

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 6, 2024
@rfcbot
Copy link

rfcbot commented Apr 6, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Apr 6, 2024
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 16, 2024
@rfcbot
Copy link

rfcbot commented Apr 16, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Apr 16, 2024
@slanterns slanterns linked a pull request Apr 16, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants