Skip to content

introduce implied_by in #[unstable] attribute#99212

Merged
bors merged 6 commits intorust-lang:masterfrom
davidtwco:partial-stability-implies
Jul 20, 2022
Merged

introduce implied_by in #[unstable] attribute#99212
bors merged 6 commits intorust-lang:masterfrom
davidtwco:partial-stability-implies

Conversation

@davidtwco
Copy link
Member

@davidtwco davidtwco commented Jul 13, 2022

Requested by the library team on Zulip.

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.

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.
        /// 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:

    /// 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

Loading
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants