Reduce the places where `stable` annotations are needed

[estebank]

Looking at the definition of std::option::Option, we have the following stable annotations:

#[derive(Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
#[stable(feature = "rust1", since = "1.0.0")]
pub enum Option<T> {
    /// No value
    #[stable(feature = "rust1", since = "1.0.0")]
    None,
    /// Some value `T`
    #[stable(feature = "rust1", since = "1.0.0")]
    Some(#[stable(feature = "rust1", since = "1.0.0")] T),
}

Is it strictly necessary for the stability checker to look at the fields of enum variants (or of structs, for that matter)?

It seems to me that MissingStabilityAnnotations could be modified to allow stability markings to flow down from at least a variant to its fields, as changing them is not a backwards compatible change and shouldn't ever happen. Even extending an existing enum would be backwards incompatible, so I would even flow from ADT attribute downwards. This would let the definition above to be:

#[derive(Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
#[stable(feature = "rust1", since = "1.0.0")]
pub enum Option<T> {
    /// No value
    None,
    /// Some value `T`
    Some(T),
}

This is normally not an issue, but with the new output in #65421 we will start showing this code to end users in errors, and it'd be nice if we could eliminate unnecessary boilerplate.

[pnkfelix]

Just as a reminder for anyone reading: private state (and state that falls under an unstable parent) does not need a stability attribute.

Here is a demo: play

But enums are implicitly public (and there's no way to opt out of that), so the proposal here does make some sense to me, at least for enums alone.

Also, if an enum were marked non-exhaustive (#44109), then of course it would be backwards compatible to add a new variant (which I what I interpreted "extending an existing enum" to mean, though perhaps that was meant to denote adding a new field to an existing variant???). So whatever we do here, it should account for that too.

[pnkfelix]

(also, we don't seem to have a tracking issue for staged_api, presumably because it is likely to remain forever an internal implementation detail? So I suspect we might consider removing T-lang from the discussion here...)

[pnkfelix]

triage: Leaving nominated for discussion. Not assigning a priority yet (I would have assigned P-medium since this is mostly internal technical debt, but the point @estebank makes about this soon become end-user visible is a little worrisome to me...)

[pnkfelix]

with the new output in #65421 we will start showing this code to end users in errors

wait, @estebank , are you sure we're going to point users to spans inside libcore and libstd? I would have assumed that such spans are not displayed in any diagnostics.

• Holy cow, we really do: https://github.com/rust-lang/rust/blob/96b83b9b7364ca3d5c52dddf8f24c9d94ec98511/src/test/ui/proc-macro/parent-source-spans.stderr#L136

[nikomatsakis]

I'd be in favor of lessening the annotation burden here, but I've not given a lot of thought, or looked for edge cases that might get overlooked.

[estebank]

@pnkfelix the PR that would make this a more pressing matter is being held up on this. I might merge the PR without pointing at std for now, but we do need to resolve this as we'll likely start pointing more and more into the stdlib, within reason.

[Centril]

But enums are implicitly public (and there's no way to opt out of that), so the proposal here does make some sense to me, at least for enums alone.

I've personally wanted to add privacy annotations to enum variants, so while the default would still be implicitly private, if we add such annotations, we'd need to remember to change the behavior here.

[Centril]

Worth noting that these stability annotations serve as documenting the history of additions. This seems relevant when considering non_exhaustive enums.

[estebank]

Currently the stable attribute is needed in 3 different levels (4 places in this case). Removing the need in at least one of them would be a win in my eyes.

[pnkfelix]

triage: leaving nominated and unprioritized.

My intention is for the team to come to a consensus as to whether we want to do this or not. I had hoped such discussion would occur during last week's T-compiler meeting, but I did not make my intent clear at that time.

Update: we subsequently discussed it at this week's meeting.

[nikomatsakis]

My opinion:

We should probably remove stability annotations from (public) enum variants.

However, I would love it if somebody from @rust-lang/libs would weigh in just to give a "seems good" sort of comment -- I'm wondering if there are scenarios they can think of where they wanted to have an enum that was stable but not its variants, and whether they feel like they'd be surprised by the change here.