Skip to content

Conversation

Coekjan
Copy link
Contributor

@Coekjan Coekjan commented May 17, 2024

This pr aims to stablize const_binary_heap_constructor.

BinaryHeap::new is able to be stablized, but BinaryHeap::new_in is not. Because the latter depends on Vec::new_in which is unstable.

The const_binary_heap_constructor feature contains the two functions, and I want to split this feature now. This pr creates const_binary_heap_new_in feature for BinaryHeap::new_in and stablizes const_binary_heap_constructor (now this original feature only contains one function).

@Coekjan Coekjan changed the title stablize const_binary_heap_constructor Stablize const_binary_heap_constructor May 17, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 17, 2024

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 17, 2024
@cuviper
Copy link
Member

cuviper commented May 17, 2024

@rustbot label -T-libs +T-libs-api

r? libs-api

@rustbot rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 17, 2024
@rustbot rustbot assigned Amanieu and unassigned cuviper May 17, 2024
@jhpratt
Copy link
Member

jhpratt commented May 20, 2024

Has FCP been completed anywhere? I don't expect this to be controversial given all similar constructors are const, but it is still required.

@Coekjan
Copy link
Contributor Author

Coekjan commented May 21, 2024

Has FCP been completed anywhere? I don't expect this to be controversial given all similar constructors are const, but it is still required.

No FCP yet. Should I do it in a new issue? Or just do it in #112353?

@jhpratt
Copy link
Member

jhpratt commented May 21, 2024

That issue is preferable to this PR, as people would already be subscribed there.

@jhpratt jhpratt added the needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. label May 21, 2024
@jhpratt
Copy link
Member

jhpratt commented May 21, 2024

@rustbot claim

r=me if and when FCP passes

@rustbot rustbot assigned jhpratt and unassigned Amanieu May 21, 2024
@Coekjan
Copy link
Contributor Author

Coekjan commented May 31, 2024

@rustbot ready

@jhpratt FCP passed

@rustbot
Copy link
Collaborator

rustbot commented May 31, 2024

Could not assign reviewer from: jhpratt.
User(s) jhpratt are either the PR author, already assigned, or on vacation, and there are no other candidates.
Use r? to specify someone else to assign.

@dtolnay
Copy link
Member

dtolnay commented May 31, 2024

@bors r=jhpratt,dtolnay

@bors
Copy link
Collaborator

bors commented May 31, 2024

📌 Commit ef23a5b has been approved by jhpratt,dtolnay

It is now in the queue for this repository.

@bors 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 May 31, 2024
@bors
Copy link
Collaborator

bors commented May 31, 2024

⌛ Testing commit ef23a5b with merge 179b6d7...

bors added a commit to rust-lang-ci/rust that referenced this pull request May 31, 2024
…,dtolnay

Stablize `const_binary_heap_constructor`

This pr aims to stablize `const_binary_heap_constructor`.

`BinaryHeap::new` is able to be stablized, but `BinaryHeap::new_in` is not. Because the latter depends on `Vec::new_in` which is unstable.

The `const_binary_heap_constructor` feature contains the two functions, and I want to split this feature now. This pr creates `const_binary_heap_new_in` feature for `BinaryHeap::new_in` and stablizes `const_binary_heap_constructor` (now this original feature only contains one function).
@bors
Copy link
Collaborator

bors commented May 31, 2024

💔 Test failed - checks-actions

@bors 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 May 31, 2024
@rust-log-analyzer

This comment has been minimized.

@jhpratt
Copy link
Member

jhpratt commented Jun 1, 2024

Looks like x fmt needs to be run.

@bors d+

@jhpratt
Copy link
Member

jhpratt commented Jun 1, 2024

@bors r- d+

@Coekjan You should be able to respond with @bors r=jhpratt once you format the code. I thought the bot usually responded when using d+. If it doesn't work, don't worry — I'll get to it soon afterwards.

@bors bors 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 Jun 1, 2024
… `const_binary_heap_new_in` for `BinaryHeap::new_in`
@Coekjan Coekjan force-pushed the const-binary-heap branch from ef23a5b to 7cee7c6 Compare June 1, 2024 04:45
@Coekjan
Copy link
Contributor Author

Coekjan commented Jun 1, 2024

@bors r=jhpratt

@bors
Copy link
Collaborator

bors commented Jun 1, 2024

@Coekjan: 🔑 Insufficient privileges: Not in reviewers

@jhpratt
Copy link
Member

jhpratt commented Jun 1, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 1, 2024

📌 Commit 7cee7c6 has been approved by jhpratt

It is now in the queue for this repository.

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 1, 2024
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 1, 2024
Stablize `const_binary_heap_constructor`

This pr aims to stablize `const_binary_heap_constructor`.

`BinaryHeap::new` is able to be stablized, but `BinaryHeap::new_in` is not. Because the latter depends on `Vec::new_in` which is unstable.

The `const_binary_heap_constructor` feature contains the two functions, and I want to split this feature now. This pr creates `const_binary_heap_new_in` feature for `BinaryHeap::new_in` and stablizes `const_binary_heap_constructor` (now this original feature only contains one function).
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 1, 2024
Stablize `const_binary_heap_constructor`

This pr aims to stablize `const_binary_heap_constructor`.

`BinaryHeap::new` is able to be stablized, but `BinaryHeap::new_in` is not. Because the latter depends on `Vec::new_in` which is unstable.

The `const_binary_heap_constructor` feature contains the two functions, and I want to split this feature now. This pr creates `const_binary_heap_new_in` feature for `BinaryHeap::new_in` and stablizes `const_binary_heap_constructor` (now this original feature only contains one function).
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 1, 2024
Stablize `const_binary_heap_constructor`

This pr aims to stablize `const_binary_heap_constructor`.

`BinaryHeap::new` is able to be stablized, but `BinaryHeap::new_in` is not. Because the latter depends on `Vec::new_in` which is unstable.

The `const_binary_heap_constructor` feature contains the two functions, and I want to split this feature now. This pr creates `const_binary_heap_new_in` feature for `BinaryHeap::new_in` and stablizes `const_binary_heap_constructor` (now this original feature only contains one function).
@jieyouxu
Copy link
Member

jieyouxu commented Jun 1, 2024

@bors r- d+

@Coekjan You should be able to respond with @bors r=jhpratt once you format the code. I thought the bot usually responded when using d+. If it doesn't work, don't worry — I'll get to it soon afterwards.

@jhpratt I think you need to spell out "delegate+", don't think "d+" is recognized

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 1, 2024
Rollup of 3 pull requests

Successful merges:

 - rust-lang#125211 (Stablize `const_binary_heap_constructor`)
 - rust-lang#125683 (Rewrite `suspicious-library`, `resolve-rename` and `incr-prev-body-beyond-eof` `run-make` tests in `rmake.rs` format)
 - rust-lang#125822 (Refactor `--print=check-cfg` test)

r? `@ghost`
`@rustbot` modify labels: rollup
@jhpratt
Copy link
Member

jhpratt commented Jun 1, 2024

Ah, okay. I went by https://bors.tech/documentation, but I also know that Rust uses its own fork. Looking at https://bors.rust-lang.org, it seems you're correct.

@bors
Copy link
Collaborator

bors commented Jun 1, 2024

⌛ Testing commit 7cee7c6 with merge 466be51...

@bors
Copy link
Collaborator

bors commented Jun 1, 2024

☀️ Test successful - checks-actions
Approved by: jhpratt
Pushing 466be51 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 1, 2024
@bors bors merged commit 466be51 into rust-lang:master Jun 1, 2024
@rustbot rustbot added this to the 1.80.0 milestone Jun 1, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (466be51): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.8%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.8% [-0.8%, -0.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.8% [-0.8%, -0.8%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 666.776s -> 668.037s (0.19%)
Artifact size: 318.89 MiB -> 318.86 MiB (-0.01%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants