Auto merge of rust-lang#117148 - dtolnay:sinceversion, r=cjgillot

Store #[stable] attribute's `since` value in structured form

Followup to rust-lang#116773 (review).

Prior to this PR, if you wrote an improper `since` version in a `stable` attribute, such as `#[stable(feature = "foo", since = "wat.0")]`, rustc would emit a diagnostic saying **_'since' must be a Rust version number, such as "1.31.0"_** and then throw out the whole `stable` attribute as if it weren't there. This strategy had 2 problems, both fixed in this PR:

1. If there was also a `#[deprecated]` attribute on the same item, rustc would want to enforce that the stabilization version is older than the deprecation version. This involved reparsing the `stable` attribute's `since` version, with a diagnostic **_invalid stability version found_** if it failed to parse. Of course this diagnostic was unreachable because an invalid `since` version would have already caused the `stable` attribute to be thrown out. This PR deletes that unreachable diagnostic.

2. By throwing out the `stable` attribute when `since` is invalid, you'd end up with a second diagnostic saying **_function has missing stability attribute_** even though your function is not missing a stability attribute. This PR preserves the `stable` attribute even when `since` cannot be parsed, avoiding the misleading second diagnostic.

Followups I plan to try next:

- Do the same for the `since` value of `#[deprecated]`.

- See whether it makes sense to also preserve `stable` and/or `unstable` attributes when they contain an invalid `feature`. What redundant/misleading diagnostics can this eliminate? What problems arise from not having a usable feature name for some API, in the situation that we're already failing compilation, so not concerned about anything that happens in downstream code?

Author: bors

compiler/rustc_attr/src/builtin.rs

@@ -13,6 +13,7 @@ use rustc_session::parse::{feature_err, ParseSess};
 use rustc_session::Session;
 use rustc_span::hygiene::Transparency;
 use rustc_span::{symbol::sym, symbol::Symbol, Span};
+use std::fmt::{self, Display};
 use std::num::NonZeroU32;
 
 use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause};
@@ -23,9 +24,10 @@ use crate::session_diagnostics::{self, IncorrectReprFormatGenericCause};
 /// For more, see [this pull request](https://github.com/rust-lang/rust/pull/100591).
 pub const VERSION_PLACEHOLDER: &str = "CURRENT_RUSTC_VERSION";
 
+pub const CURRENT_RUSTC_VERSION: &str = env!("CFG_RELEASE");
+
 pub fn rust_version_symbol() -> Symbol {
-    let version = option_env!("CFG_RELEASE").unwrap_or("<current>");
-    Symbol::intern(&version)
+    Symbol::intern(CURRENT_RUSTC_VERSION)
 }
 
 pub fn is_builtin_attr(attr: &Attribute) -> bool {
@@ -144,13 +146,24 @@ pub enum StabilityLevel {
     /// `#[stable]`
     Stable {
         /// Rust release which stabilized this feature.
-        since: Symbol,
+        since: Since,
         /// Is this item allowed to be referred to on stable, despite being contained in unstable
         /// modules?
         allowed_through_unstable_modules: bool,
     },
 }
 
+/// Rust release in which a feature is stabilized.
+#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)]
+#[derive(HashStable_Generic)]
+pub enum Since {
+    Version(Version),
+    /// Stabilized in the upcoming version, whatever number that is.
+    Current,
+    /// Failed to parse a stabilization version.
+    Err,
+}
+
 impl StabilityLevel {
     pub fn is_unstable(&self) -> bool {
         matches!(self, StabilityLevel::Unstable { .. })
@@ -372,22 +385,24 @@ fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabilit
 
     let since = if let Some(since) = since {
         if since.as_str() == VERSION_PLACEHOLDER {
-            Ok(rust_version_symbol())
-        } else if parse_version(since.as_str(), false).is_some() {
-            Ok(since)
+            Since::Current
+        } else if let Some(version) = parse_version(since.as_str(), false) {
+            Since::Version(version)
         } else {
-            Err(sess.emit_err(session_diagnostics::InvalidSince { span: attr.span }))
+            sess.emit_err(session_diagnostics::InvalidSince { span: attr.span });
+            Since::Err
         }
     } else {
-        Err(sess.emit_err(session_diagnostics::MissingSince { span: attr.span }))
+        sess.emit_err(session_diagnostics::MissingSince { span: attr.span });
+        Since::Err
     };
 
-    match (feature, since) {
-        (Ok(feature), Ok(since)) => {
+    match feature {
+        Ok(feature) => {
             let level = StabilityLevel::Stable { since, allowed_through_unstable_modules: false };
             Some((feature, level))
         }
-        (Err(ErrorGuaranteed { .. }), _) | (_, Err(ErrorGuaranteed { .. })) => None,
+        Err(ErrorGuaranteed { .. }) => None,
     }
 }
 
@@ -556,11 +571,12 @@ fn gate_cfg(gated_cfg: &GatedCfg, cfg_span: Span, sess: &ParseSess, features: &F
     }
 }
 
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-struct Version {
-    major: u16,
-    minor: u16,
-    patch: u16,
+#[derive(Encodable, Decodable, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(HashStable_Generic)]
+pub struct Version {
+    pub major: u16,
+    pub minor: u16,
+    pub patch: u16,
 }
 
 fn parse_version(s: &str, allow_appendix: bool) -> Option<Version> {
@@ -576,6 +592,12 @@ fn parse_version(s: &str, allow_appendix: bool) -> Option<Version> {
     Some(Version { major, minor, patch })
 }
 
+impl Display for Version {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(formatter, "{}.{}.{}", self.major, self.minor, self.patch)
+    }
+}
+
 /// Evaluate a cfg-like condition (with `any` and `all`), using `eval` to
 /// evaluate individual items.
 pub fn eval_condition(
@@ -609,7 +631,7 @@ pub fn eval_condition(
                 sess.emit_warning(session_diagnostics::UnknownVersionLiteral { span: *span });
                 return false;
             };
-            let rustc_version = parse_version(env!("CFG_RELEASE"), true).unwrap();
+            let rustc_version = parse_version(CURRENT_RUSTC_VERSION, true).unwrap();
 
             // See https://github.com/rust-lang/rust/issues/64796#issuecomment-640851454 for details
             if sess.assume_incomplete_release {

compiler/rustc_passes/messages.ftl

@@ -402,11 +402,6 @@ passes_invalid_macro_export_arguments = `{$name}` isn't a valid `#[macro_export]
 
 passes_invalid_macro_export_arguments_too_many_items = `#[macro_export]` can only take 1 or 0 arguments
 
-passes_invalid_stability =
-    invalid stability version found
-    .label = invalid stability version
-    .item = the stability attribute annotates this item
-
 passes_lang_item_fn_with_target_feature =
     `{$name}` language item function is not allowed to have `#[target_feature]`
     .label = `{$name}` language item function is not allowed to have `#[target_feature]`

compiler/rustc_passes/src/errors.rs

@@ -1504,16 +1504,6 @@ pub struct UselessStability {
     pub item_sp: Span,
 }
 
-#[derive(Diagnostic)]
-#[diag(passes_invalid_stability)]
-pub struct InvalidStability {
-    #[primary_span]
-    #[label]
-    pub span: Span,
-    #[label(passes_item)]
-    pub item_sp: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag(passes_cannot_stabilize_deprecated)]
 pub struct CannotStabilizeDeprecated {

compiler/rustc_passes/src/stability.rs

@@ -3,7 +3,7 @@
 
 use crate::errors;
 use rustc_attr::{
-    self as attr, rust_version_symbol, ConstStability, Stability, StabilityLevel, Unstable,
+    self as attr, rust_version_symbol, ConstStability, Since, Stability, StabilityLevel, Unstable,
     UnstableReason, VERSION_PLACEHOLDER,
 };
 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
@@ -226,37 +226,43 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
             if let (&Some(dep_since), &attr::Stable { since: stab_since, .. }) =
                 (&depr.as_ref().and_then(|(d, _)| d.since), &stab.level)
             {
-                // Explicit version of iter::order::lt to handle parse errors properly
-                for (dep_v, stab_v) in
-                    iter::zip(dep_since.as_str().split('.'), stab_since.as_str().split('.'))
-                {
-                    match stab_v.parse::<u64>() {
-                        Err(_) => {
-                            self.tcx.sess.emit_err(errors::InvalidStability { span, item_sp });
-                            break;
-                        }
-                        Ok(stab_vp) => match dep_v.parse::<u64>() {
-                            Ok(dep_vp) => match dep_vp.cmp(&stab_vp) {
-                                Ordering::Less => {
-                                    self.tcx.sess.emit_err(errors::CannotStabilizeDeprecated {
-                                        span,
-                                        item_sp,
-                                    });
+                match stab_since {
+                    Since::Current => {
+                        self.tcx.sess.emit_err(errors::CannotStabilizeDeprecated { span, item_sp });
+                    }
+                    Since::Version(stab_since) => {
+                        // Explicit version of iter::order::lt to handle parse errors properly
+                        for (dep_v, stab_v) in iter::zip(
+                            dep_since.as_str().split('.'),
+                            [stab_since.major, stab_since.minor, stab_since.patch],
+                        ) {
+                            match dep_v.parse::<u64>() {
+                                Ok(dep_vp) => match dep_vp.cmp(&u64::from(stab_v)) {
+                                    Ordering::Less => {
+                                        self.tcx.sess.emit_err(errors::CannotStabilizeDeprecated {
+                                            span,
+                                            item_sp,
+                                        });
+                                        break;
+                                    }
+                                    Ordering::Equal => continue,
+                                    Ordering::Greater => break,
+                                },
+                                Err(_) => {
+                                    if dep_v != "TBD" {
+                                        self.tcx.sess.emit_err(errors::InvalidDeprecationVersion {
+                                            span,
+                                            item_sp,
+                                        });
+                                    }
                                     break;
                                 }
-                                Ordering::Equal => continue,
-                                Ordering::Greater => break,
-                            },
-                            Err(_) => {
-                                if dep_v != "TBD" {
-                                    self.tcx.sess.emit_err(errors::InvalidDeprecationVersion {
-                                        span,
-                                        item_sp,
-                                    });
-                                }
-                                break;
                             }
-                        },
+                        }
+                    }
+                    Since::Err => {
+                        // An error already reported. Assume the unparseable stabilization
+                        // version is older than the deprecation version.
                     }
                 }
             }

src/librustdoc/clean/types.rs

@@ -12,7 +12,7 @@ use thin_vec::ThinVec;
 
 use rustc_ast as ast;
 use rustc_ast_pretty::pprust;
-use rustc_attr::{ConstStability, Deprecation, Stability, StabilityLevel};
+use rustc_attr::{ConstStability, Deprecation, Since, Stability, StabilityLevel};
 use rustc_const_eval::const_eval::is_unstable_const_fn;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
@@ -585,14 +585,14 @@ impl Item {
         })
     }
 
-    pub(crate) fn stable_since(&self, tcx: TyCtxt<'_>) -> Option<Symbol> {
+    pub(crate) fn stable_since(&self, tcx: TyCtxt<'_>) -> Option<Since> {
         match self.stability(tcx)?.level {
             StabilityLevel::Stable { since, .. } => Some(since),
             StabilityLevel::Unstable { .. } => None,
         }
     }
 
-    pub(crate) fn const_stable_since(&self, tcx: TyCtxt<'_>) -> Option<Symbol> {
+    pub(crate) fn const_stable_since(&self, tcx: TyCtxt<'_>) -> Option<Since> {
         match self.const_stability(tcx)?.level {
             StabilityLevel::Stable { since, .. } => Some(since),
             StabilityLevel::Unstable { .. } => None,

src/librustdoc/html/render/mod.rs

@@ -48,7 +48,7 @@ use std::str;
 use std::string::ToString;
 
 use askama::Template;
-use rustc_attr::{ConstStability, Deprecation, StabilityLevel};
+use rustc_attr::{ConstStability, Deprecation, Since, StabilityLevel, CURRENT_RUSTC_VERSION};
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir::def_id::{DefId, DefIdSet};
@@ -911,13 +911,17 @@ fn assoc_method(
 /// consequence of the above rules.
 fn render_stability_since_raw_with_extra(
     w: &mut Buffer,
-    ver: Option<Symbol>,
+    ver: Option<Since>,
     const_stability: Option<ConstStability>,
-    containing_ver: Option<Symbol>,
-    containing_const_ver: Option<Symbol>,
+    containing_ver: Option<Since>,
+    containing_const_ver: Option<Since>,
     extra_class: &str,
 ) -> bool {
-    let stable_version = ver.filter(|inner| !inner.is_empty() && Some(*inner) != containing_ver);
+    let stable_version = if ver != containing_ver && let Some(ver) = &ver {
+        since_to_string(ver)
+    } else {
+        None
+    };
 
     let mut title = String::new();
     let mut stability = String::new();
@@ -931,7 +935,8 @@ fn render_stability_since_raw_with_extra(
         Some(ConstStability { level: StabilityLevel::Stable { since, .. }, .. })
             if Some(since) != containing_const_ver =>
         {
-            Some((format!("const since {since}"), format!("const: {since}")))
+            since_to_string(&since)
+                .map(|since| (format!("const since {since}"), format!("const: {since}")))
         }
         Some(ConstStability { level: StabilityLevel::Unstable { issue, .. }, feature, .. }) => {
             let unstable = if let Some(n) = issue {
@@ -971,13 +976,21 @@ fn render_stability_since_raw_with_extra(
     !stability.is_empty()
 }
 
+fn since_to_string(since: &Since) -> Option<String> {
+    match since {
+        Since::Version(since) => Some(since.to_string()),
+        Since::Current => Some(CURRENT_RUSTC_VERSION.to_owned()),
+        Since::Err => None,
+    }
+}
+
 #[inline]
 fn render_stability_since_raw(
     w: &mut Buffer,
-    ver: Option<Symbol>,
+    ver: Option<Since>,
     const_stability: Option<ConstStability>,
-    containing_ver: Option<Symbol>,
-    containing_const_ver: Option<Symbol>,
+    containing_ver: Option<Since>,
+    containing_const_ver: Option<Since>,
 ) -> bool {
     render_stability_since_raw_with_extra(
         w,

src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs

@@ -5,6 +5,7 @@
 
 use crate::msrvs::Msrv;
 use hir::LangItem;
+use rustc_attr::{Since, CURRENT_RUSTC_VERSION};
 use rustc_const_eval::transform::check_consts::ConstCx;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
@@ -370,19 +371,24 @@ fn is_const_fn(tcx: TyCtxt<'_>, def_id: DefId, msrv: &Msrv) -> bool {
                 // 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 the `-dev` version number so we have to strip it off.
-                let short_version = since
-                    .as_str()
-                    .split('-')
-                    .next()
-                    .expect("rustc_attr::StabilityLevel::Stable::since` is empty");
+                let const_stab_rust_version = match since {
+                    Since::Version(version) => RustcVersion::new(
+                        u32::from(version.major),
+                        u32::from(version.minor),
+                        u32::from(version.patch),
+                    ),
+                    Since::Current => {
+                        // HACK(nilstrieb): CURRENT_RUSTC_VERSION can return versions like 1.66.0-dev.
+                        // `rustc-semver` doesn't accept the `-dev` version number so we have to strip it off.
+                        let short_version = CURRENT_RUSTC_VERSION.split('-').next().unwrap();
+                        RustcVersion::parse(short_version).unwrap_or_else(|err| {
+                            panic!("`rustc_attr::StabilityLevel::Stable::since` is ill-formatted: `{CURRENT_RUSTC_VERSION}`, {err:?}")
+                        })
+                    },
+                    Since::Err => return false,
+                };
 
-                let since = rustc_span::Symbol::intern(short_version);
-
-                msrv.meets(RustcVersion::parse(since.as_str()).unwrap_or_else(|err| {
-                    panic!("`rustc_attr::StabilityLevel::Stable::since` is ill-formatted: `{since}`, {err:?}")
-                }))
+                msrv.meets(const_stab_rust_version)
             } else {
                 // Unstable const fn with the feature enabled.
                 msrv.current().is_none()

tests/rustdoc/html-no-source.rs

@@ -11,20 +11,20 @@
 // @files 'src/foo' '[]'
 
 // @has foo/fn.foo.html
-// @has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0 · '
-// @!has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0 · source · '
+// @has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0.0 · '
+// @!has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0.0 · source · '
 #[stable(feature = "bar", since = "1.0")]
 pub fn foo() {}
 
 // @has foo/struct.Bar.html
-// @has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0 · '
-// @!has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0 · source · '
+// @has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0.0 · '
+// @!has - '//div[@class="main-heading"]/*[@class="out-of-band"]' '1.0.0 · source · '
 #[stable(feature = "bar", since = "1.0")]
 pub struct Bar;
 
 impl Bar {
-    // @has - '//*[@id="method.bar"]/*[@class="since rightside"]' '2.0'
-    // @!has - '//*[@id="method.bar"]/*[@class="rightside"]' '2.0 ·'
+    // @has - '//*[@id="method.bar"]/*[@class="since rightside"]' '2.0.0'
+    // @!has - '//*[@id="method.bar"]/*[@class="rightside"]' '2.0.0 ·'
     #[stable(feature = "foobar", since = "2.0")]
     pub fn bar() {}
 }