Constification of BinaryHeap construction

[terrarier2111]

Is there any reason this isn't const yet? This should only depend on vec's construction being const which it is.

[est31]

All the other collections already have const constructors (except for the ones for which it's hard, cc #112354). So I think it makes a lot of sense to make this const as well.

There was an entire ACP for VecDequeue::new(). I don't know if one is needed for BinaryHeap as well. On the other hand, the function became instantly const-stable, there was no period where it was unstably const, see #105072.

[tsemo4917]

impl<T: Ord> BinaryHeap<T> {
    pub fn new() -> BinaryHeap<T> {
        BinaryHeap { data: vec![] }
    }
}

BinaryHeap constructor is only depends on Vec constructor, and Vec::new() has been a const function since 1.39.0.

Constification of BinaryHeap constructor will be helpful when a static variable is of the BinaryHeap type.

@dtolnay

[dtolnay]

Please send a PR.

[Coekjan]

I also met this issue and I am trying to address it with #117316

[Coekjan]

The feature const_binary_heap_constructor was added to nightly.

Due to the unstable feature (allocator_api, issue: #32838 ), this feature is not able to be stablized now.

[LGFae]

Hello, @Coekjan, according to what you said in #117316, it seems that the new function could be stabilized, even without the allocator_api being stable. Any news on that front?

[Coekjan]

Hello, @Coekjan, according to what you said in #117316, it seems that the new function could be stabilized, even without the allocator_api being stable. Any news on that front?

I am a little busy this week, and I will try to see how to stablize it later.

[Coekjan]

Hello, @Coekjan, according to what you said in #117316, it seems that the new function could be stabilized, even without the allocator_api being stable. Any news on that front?

See #125211 I try to place the two functions into two different features, and stablize BinaryHeap::new.

[jhpratt]

Nomination for discussion to stabilize. I think it's quite straightforward and has plenty of precedent.

[dtolnay]

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

const on the already-stable constructor for BinaryHeap.

impl<T: Ord> BinaryHeap<T> {
    #[must_use]
    pub const fn new() -> Self;
}

[rfcbot]

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

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

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]

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

[rfcbot]

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.

[scottmcm]

There was an entire ACP for VecDequeue::new(). I don't know if one is needed for BinaryHeap as well.

Notably, VecDeque was more interesting, because making it const implied making it non-allocating, and thus changing the existing approach for the whole type. No such complication exists for BinaryHeap.

[est31]

Given that #125211 has been merged, and BinaryHeap::new is const-stable now, this tracking issue is now only for const-stability of BinaryHeap::new_in.

Maybe this should be indicated at the top of the issue? Or should we make a new tracking issue?

[jhpratt]

@est31 Is there typically a tracking issue for const-stabilizing something that is itself unstable? I didn't think so, so I would actually recommend closing this.

[Coekjan]

If it is ok, let me open a new issue to track BinaryHeap::new_in, and I will update the number of the tracking issue in the code.