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

Stabilize const BTree{Map,Set}::new #102197

Merged
merged 2 commits into from
Sep 26, 2022
Merged

Conversation

Noratrieb
Copy link
Member

The FCP was completed in #71835.

Since len and is_empty are not const stable yet, this also creates a new feature for them since they previously used the same const_btree_new feature.

@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 23, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 23, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 23, 2022
@Noratrieb
Copy link
Member Author

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

@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 Sep 23, 2022
Since `len` and `is_empty` are not const stable yet, this also
creates a new feature for them since they previously used the same
`const_btree_new` feature.
Since clippy can use a projects MSRV for its lints, it might not want
to consider functions as const stable if they have been added lately.

Functions that have been stabilized this version use
CURRENT_RUSTC_VERSION as their version, which gets then turned into the
current version, which might be something like `1.66.0-dev`. The version
parser cannot deal with this version, so it has to be stripped off.
@@ -367,10 +367,21 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: Option<RustcVersion>) -> bo
// Checking MSRV is manually necessary because `rustc` has no such concept. This entire
// function could be removed if `rustc` provided a MSRV-aware version of `is_const_fn`.
// as a part of an unimplemented MSRV check https://github.com/rust-lang/rust/issues/65262.

// HACK(nilstrieb): CURRENT_RUSTC_VERSION can return versions like 1.66.0-dev. `rustc-semver` doesn't accept
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@est31 might be good to followup on this and figure out what's happening here -- we probably shouldn't be returning -dev in the version, since we don't include -nightly for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the -dev is being returned because the CFG_VERSION env var contains it. That env var itself is set by the bootstrap tooling (and it also adds -nightly postfixes FTR). I originally went with the most simple code and didn't strip the postfix because that would have involved parsing the version number (like this code is doing). I'd thought it wouldn't be much of a problem, it seems that most of the tooling could handle the postfix.

I like the -nightly postfix because it gives it a notion of preliminarity: attention that compiler has not been released yet, the stabilization might be reverted (like never has been multiple times, or let chains very recently). Also, sometimes that number can lie: for example right now the cast_const function is shown to be stabilized on 1.66.0-nightly in the nightly docs:

Screenshot_20220926_083953

Even though it's actually been stabilized in 1.65.0, and beta already correctly shows this:

Screenshot_20220926_084224

Admittedly, this only occurs during a ~one week period during the release cycle, in this particular instance because #102051 hasn't been merged yet to master, which changes the placeholder to 1.65.0. But I think the -nightly postfix helps here in this instance, makes people more cautious that it might be wrong. Or idk.

So I'm not that unhappy with the current postfix, but I'm willing to remove it, if it's really not wanted. #101772 has introduced the rust_version_symbol function, so there is a centralized place to change it, to add code not dissimilar from this one.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup since FCP completed; we can follow-up on the comment/clippy hack at a later point in a separate PR.

@bors
Copy link
Contributor

bors commented Sep 26, 2022

📌 Commit 66484c0 has been approved by Mark-Simulacrum

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 Sep 26, 2022
fee1-dead added a commit to fee1-dead-contrib/rust that referenced this pull request Sep 26, 2022
…lacrum

Stabilize const `BTree{Map,Set}::new`

The FCP was completed in rust-lang#71835.

Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 26, 2022
…fee1-dead

Rollup of 5 pull requests

Successful merges:

 - rust-lang#102143 (Recover from struct nested in struct)
 - rust-lang#102178 (bootstrap: the backtrace feature is stable, no need to allow it any more)
 - rust-lang#102197 (Stabilize const `BTree{Map,Set}::new`)
 - rust-lang#102267 (Don't set RUSTC in the bootstrap build script)
 - rust-lang#102270 (Remove benches from `rustc_middle`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 804c2c1 into rust-lang:master Sep 26, 2022
@rustbot rustbot added this to the 1.66.0 milestone Sep 26, 2022
@Noratrieb Noratrieb deleted the const-new-🌲 branch September 26, 2022 08:24
@WaffleLapkin
Copy link
Member

Since len and is_empty are not const stable yet, this also creates a new feature for them since they previously used the same const_btree_new feature.

I don't think that's right. When we split features we change the feature gate for stabilized part, not the still unstable part. This is because we don't want to break code that was written for older nightlies and used old gate for still unstable methods.

@est31
Copy link
Member

est31 commented Oct 1, 2022

IDK if such a policy exists or not, but you could maybe file a PR to add an implied_by tag? See #99212 which was precisely about this scenario.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 2, 2022
… r=Mark-Simulacrum

Make `feature(const_btree_len)` implied by `feature(const_btree_new)`

...this should fix code that used the old feature that was changed in rust-lang#102197

cc `@davidtwco` it seems like tidy doesn't check `implied_by`, should it?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 2, 2022
… r=Mark-Simulacrum

Make `feature(const_btree_len)` implied by `feature(const_btree_new)`

...this should fix code that used the old feature that was changed in rust-lang#102197

cc ``@davidtwco`` it seems like tidy doesn't check `implied_by`, should it?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 2, 2022
… r=Mark-Simulacrum

Make `feature(const_btree_len)` implied by `feature(const_btree_new)`

...this should fix code that used the old feature that was changed in rust-lang#102197

cc ```@davidtwco``` it seems like tidy doesn't check `implied_by`, should it?
flip1995 pushed a commit to flip1995/rust that referenced this pull request Oct 6, 2022
…lacrum

Stabilize const `BTree{Map,Set}::new`

The FCP was completed in rust-lang#71835.

Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
… r=Mark-Simulacrum

Make `feature(const_btree_len)` implied by `feature(const_btree_new)`

...this should fix code that used the old feature that was changed in rust-lang#102197

cc ```@davidtwco``` it seems like tidy doesn't check `implied_by`, should it?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.

7 participants