[DO NOT MERGE] perf-test for #99212 #99897
Conversation
Original message: Rollup merge of rust-lang#99212 - davidtwco:partial-stability-implies, r=michaelwoerister introduce `implied_by` in `#[unstable]` attribute Requested by the library team [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/better.20support.20for.20partial.20stabilizations/near/285581519). If part of a feature is stabilized and a new feature is added for the remaining parts, then the `implied_by` meta-item can be added to `#[unstable]` to indicate which now-stable feature was used previously. ```diagnostic error: the feature `foo` has been partially stabilized since 1.62.0 and is succeeded by the feature `foobar` --> $DIR/stability-attribute-implies-using-unstable.rs:3:12 | LL | #![feature(foo)] | ^^^ | note: the lint level is defined here --> $DIR/stability-attribute-implies-using-stable.rs:2:9 | LL | #![deny(stable_features)] | ^^^^^^^^^^^^^^^ help: if you are using features which are still unstable, change to using `foobar` | LL | #![feature(foobar)] | ~~~~~~ help: if you are using features which are now stable, remove this line | LL - #![feature(foo)] | ``` When a `#![feature(..)]` attribute still exists for the now-stable attribute, then there this has two effects: - There will not be an stability error for uses of items from the implied feature which are still unstable (until the `#![feature(..)]` is removed or updated to the new feature). - There will be an improved diagnostic for the remaining use of the feature attribute for the now-stable feature. ```rust /// If part of a feature is stabilized and a new feature is added for the remaining parts, /// then the `implied_by` attribute is used to indicate which now-stable feature previously /// contained a item. /// /// ```pseudo-Rust /// #[unstable(feature = "foo", issue = "...")] /// fn foo() {} /// #[unstable(feature = "foo", issue = "...")] /// fn foobar() {} /// ``` /// /// ...becomes... /// /// ```pseudo-Rust /// #[stable(feature = "foo", since = "1.XX.X")] /// fn foo() {} /// #[unstable(feature = "foobar", issue = "...", implied_by = "foo")] /// fn foobar() {} /// ``` ``` In the Zulip discussion, this was envisioned as `implies` on `#[stable]` but I went with `implied_by` on `#[unstable]` because it means that only the unstable attribute needs to be changed in future, not the new stable attribute, which seems less error-prone. It also isn't particularly feasible for me to detect whether items from the implied feature are used and then only suggest updating _or_ removing the `#![feature(..)]` as appropriate, so I always do both. There's some new information in the cross-crate metadata as a result of this change, that's a little unfortunate, but without requiring that the `#[unstable]` and `#[stable]` attributes both contain the implication information, it's necessary: ```rust /// This mapping is necessary unless both the `#[stable]` and `#[unstable]` attributes should /// specify their implications (both `implies` and `implied_by`). If only one of the two /// attributes do (as in the current implementation, `implied_by` in `#[unstable]`), then this /// mapping is necessary for diagnostics. When a "unnecessary feature attribute" error is /// reported, only the `#[stable]` attribute information is available, so the map is necessary /// to know that the feature implies another feature. If it were reversed, and the `#[stable]` /// attribute had an `implies` meta item, then a map would be necessary when avoiding a "use of /// unstable feature" error for a feature that was implied. ``` I also change some comments to documentation comments in the compiler, add a helper for going from a `Span` to a `Span` for the entire line, and fix a incorrect part of the pre-existing stability attribute diagnostics. cc `@yaahc`
rustbot
added
T-bootstrap
|
Some changes occurred in diagnostic error codes A change occurred in the Ayu theme. cc @Cldfire Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred in src/librustdoc/clean/types.rs cc @camelid Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @Folyd, @jsha Some changes occurred in need_type_info.rs cc @lcnr Some changes occurred in HTML/CSS themes. Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred in const_evaluatable.rs cc @lcnr Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
The Miri submodule was changed cc @rust-lang/miri Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_gcc cc @antoyo Some changes occurred in src/tools/cargo cc @ehuss |
|
@bors try @rust-timer queue The try commit's (master) parent should be ea6ab1b. If it isn't, then please:
You do not need to reinvoke the queue command as long as the perf build hasn't yet started. |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 54fff5c with merge 135eb0df1bb9ed2170cc2348a964dc1ef7ebbd2b... |
|
Sorry for the pings everyone. I'm trying out the functionality outlined here for automatically unrolling a rollup and testing one of the PRs for perf regressions. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☀️ Try build successful - checks-actions |
|
The rollup changed 60 files or so, why does this PR change >2000? Looks like something went wrong...?
|
|
@RalfJung the strategy employed by the automation is to first revert back to the master at the time the rollup was created and then to apply the single merge commit of the PR being tested for perf regressions. The strategy is not to just revert the rollup itself. That explains the large amount of changes. There have been over 2,900 files changed in master since the master at the time of the rollup. |
|
Talking this over with @Mark-Simulacrum - it seems we're using the wrong master commit - we're using the sha in master at rollup creation rather than the sha at rollup merge. This may be the reason CI is failing but we're not sure. I'm going to close this for now. We're going to experiment with a different approach that optimistically builds artifacts that are testable by perf.rlo for every commit in a rollup. We can then build a command that kicks off a perf run for that artifact (initiated by a human who is investigating a rollup issue). |
|
@davidtwco I wouldn't wait on me - I'm trying to find a general solution that will likely take a bit of time. It's probably more productive for #99212 to do the revert manually. |
This is an automatically generated pull request (from here) to run perf tests for #99212 which merged in a rollup.
r? @ghost