Skip to content

Commit

Permalink
Auto merge of #117148 - dtolnay:sinceversion, r=cjgillot
Browse files Browse the repository at this point in the history
Store #[stable] attribute's `since` value in structured form

Followup to #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?
  • Loading branch information
bors committed Oct 26, 2023
2 parents ccb160d + 1a9ea1f commit 104ac7b
Show file tree
Hide file tree
Showing 12 changed files with 140 additions and 121 deletions.
56 changes: 39 additions & 17 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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 {
Expand Down Expand Up @@ -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 { .. })
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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> {
Expand All @@ -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(
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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]`
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
64 changes: 35 additions & 29 deletions compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
31 changes: 22 additions & 9 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 18 additions & 12 deletions src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions tests/rustdoc/html-no-source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
}
Loading

0 comments on commit 104ac7b

Please sign in to comment.