-
Notifications
You must be signed in to change notification settings - Fork 13k
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
The head ref may contain hidden characters: "const-new-\u{1F332}"
Conversation
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 Examples of
|
(rust-highfive has picked a reviewer for you, use r? to override) |
@rustbot label +T-libs-api -T-libs |
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.
70217fc
to
66484c0
Compare
@@ -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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
Even though it's actually been stabilized in 1.65.0
, and beta already correctly shows this:
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.
@bors r+ rollup since FCP completed; we can follow-up on the comment/clippy hack at a later point in a separate PR. |
…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.
…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
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. |
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. |
… 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?
… 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?
… 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?
…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.
… 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?
The FCP was completed in #71835.
Since
len
andis_empty
are not const stable yet, this also creates a new feature for them since they previously used the sameconst_btree_new
feature.