-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store #[deprecated] attribute's since
value in parsed form
#117377
Changes from 3 commits
1e5b2da
5c7cf83
2fe7d17
8afb40b
1e10fe9
b106167
c523672
e8868af
dccf10e
8b8906b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ use rustc_session::parse::{feature_err, ParseSess}; | |
use rustc_session::{RustcVersion, 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}; | ||
|
@@ -139,7 +140,7 @@ pub enum StabilityLevel { | |
/// `#[stable]` | ||
Stable { | ||
/// Rust release which stabilized this feature. | ||
since: Since, | ||
since: StableSince, | ||
/// Is this item allowed to be referred to on stable, despite being contained in unstable | ||
/// modules? | ||
allowed_through_unstable_modules: bool, | ||
|
@@ -149,7 +150,7 @@ pub enum StabilityLevel { | |
/// Rust release in which a feature is stabilized. | ||
#[derive(Encodable, Decodable, PartialEq, Copy, Clone, Debug, Eq, Hash)] | ||
#[derive(HashStable_Generic)] | ||
pub enum Since { | ||
pub enum StableSince { | ||
Version(RustcVersion), | ||
/// Stabilized in the upcoming version, whatever number that is. | ||
Current, | ||
|
@@ -378,16 +379,16 @@ fn parse_stability(sess: &Session, attr: &Attribute) -> Option<(Symbol, Stabilit | |
|
||
let since = if let Some(since) = since { | ||
if since.as_str() == VERSION_PLACEHOLDER { | ||
Since::Current | ||
StableSince::Current | ||
} else if let Some(version) = parse_version(since) { | ||
Since::Version(version) | ||
StableSince::Version(version) | ||
} else { | ||
sess.emit_err(session_diagnostics::InvalidSince { span: attr.span }); | ||
Since::Err | ||
StableSince::Err | ||
} | ||
} else { | ||
sess.emit_err(session_diagnostics::MissingSince { span: attr.span }); | ||
Since::Err | ||
StableSince::Err | ||
}; | ||
|
||
match feature { | ||
|
@@ -720,17 +721,37 @@ pub fn eval_condition( | |
|
||
#[derive(Copy, Debug, Encodable, Decodable, Clone, HashStable_Generic)] | ||
pub struct Deprecation { | ||
pub since: Option<Symbol>, | ||
pub since: Option<DeprecatedSince>, | ||
/// The note to issue a reason. | ||
pub note: Option<Symbol>, | ||
/// A text snippet used to completely replace any use of the deprecated item in an expression. | ||
/// | ||
/// This is currently unstable. | ||
pub suggestion: Option<Symbol>, | ||
} | ||
|
||
/// Release in which an API is deprecated. | ||
#[derive(Copy, Debug, Encodable, Decodable, Clone, HashStable_Generic)] | ||
pub enum DeprecatedSince { | ||
RustcVersion(RustcVersion), | ||
/// Deprecated in the future ("to be determined"). | ||
Future, | ||
/// `feature(staged_api)` is off, or it's on but the deprecation version | ||
/// cannot be parsed as a RustcVersion. In the latter case, an error has | ||
/// already been emitted. In the former case, deprecation versions outside | ||
/// the standard library are allowed to be arbitrary strings, for better or | ||
/// worse. | ||
Symbol(Symbol), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This variant name is not very descriptive? Custom ? UserDefined ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you like |
||
} | ||
|
||
/// Whether to treat the since attribute as being a Rust version identifier | ||
/// (rather than an opaque string). | ||
pub is_since_rustc_version: bool, | ||
impl Display for DeprecatedSince { | ||
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
match self { | ||
DeprecatedSince::RustcVersion(since) => Display::fmt(since, formatter), | ||
DeprecatedSince::Future => formatter.write_str("TBD"), | ||
DeprecatedSince::Symbol(since) => Display::fmt(since, formatter), | ||
} | ||
} | ||
} | ||
|
||
/// Finds the deprecation attribute. `None` if none exists. | ||
|
@@ -839,22 +860,30 @@ pub fn find_deprecation( | |
} | ||
} | ||
|
||
if is_rustc { | ||
if since.is_none() { | ||
sess.emit_err(session_diagnostics::MissingSince { span: attr.span }); | ||
continue; | ||
let since = if let Some(since) = since { | ||
if since.as_str() == "TBD" { | ||
Some(DeprecatedSince::Future) | ||
} else if !is_rustc { | ||
Some(DeprecatedSince::Symbol(since)) | ||
} else if let Some(version) = parse_version(since) { | ||
Some(DeprecatedSince::RustcVersion(version)) | ||
} else { | ||
sess.emit_err(session_diagnostics::InvalidSince { span: attr.span }); | ||
Some(DeprecatedSince::Symbol(since)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not convinced that reusing this variant for both purposes is useful. What about an Err variant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 I have added |
||
} | ||
} else if is_rustc { | ||
sess.emit_err(session_diagnostics::MissingSince { span: attr.span }); | ||
continue; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we stop discarding the attribute on missing since? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good catch. Fixed. I chose to use |
||
} else { | ||
None | ||
}; | ||
|
||
if note.is_none() { | ||
sess.emit_err(session_diagnostics::MissingNote { span: attr.span }); | ||
continue; | ||
} | ||
if is_rustc && note.is_none() { | ||
sess.emit_err(session_diagnostics::MissingNote { span: attr.span }); | ||
continue; | ||
} | ||
|
||
depr = Some(( | ||
Deprecation { since, note, suggestion, is_since_rustc_version: is_rustc }, | ||
attr.span, | ||
)); | ||
depr = Some((Deprecation { since, note, suggestion }, attr.span)); | ||
} | ||
|
||
depr | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,9 @@ pub use self::StabilityLevel::*; | |
|
||
use crate::ty::{self, TyCtxt}; | ||
use rustc_ast::NodeId; | ||
use rustc_attr::{self as attr, ConstStability, DefaultBodyStability, Deprecation, Stability}; | ||
use rustc_attr::{ | ||
self as attr, ConstStability, DefaultBodyStability, DeprecatedSince, Deprecation, Stability, | ||
}; | ||
use rustc_data_structures::fx::FxHashMap; | ||
use rustc_errors::{Applicability, Diagnostic}; | ||
use rustc_feature::GateIssue; | ||
|
@@ -126,36 +128,14 @@ pub fn report_unstable( | |
/// Checks whether an item marked with `deprecated(since="X")` is currently | ||
/// deprecated (i.e., whether X is not greater than the current rustc version). | ||
pub fn deprecation_in_effect(depr: &Deprecation) -> bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this be made an inherent method on Deprecation or DeprecatedSince? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done; added |
||
let is_since_rustc_version = depr.is_since_rustc_version; | ||
let since = depr.since.as_ref().map(Symbol::as_str); | ||
|
||
if !is_since_rustc_version { | ||
match depr.since { | ||
Some(DeprecatedSince::RustcVersion(since)) => since <= RustcVersion::CURRENT, | ||
Some(DeprecatedSince::Future) => false, | ||
// The `since` field doesn't have semantic purpose without `#![staged_api]`. | ||
return true; | ||
Some(DeprecatedSince::Symbol(_)) => true, | ||
// Assume deprecation is in effect if "since" field is missing. | ||
None => true, | ||
} | ||
|
||
if let Some(since) = since { | ||
if since == "TBD" { | ||
return false; | ||
} | ||
|
||
// We ignore non-integer components of the version (e.g., "nightly"). | ||
let since: Vec<u16> = | ||
since.split(|c| c == '.' || c == '-').flat_map(|s| s.parse()).collect(); | ||
|
||
// We simply treat invalid `since` attributes as relating to a previous | ||
// Rust version, thus always displaying the warning. | ||
if since.len() != 3 { | ||
return true; | ||
} | ||
|
||
let rustc = RustcVersion::CURRENT; | ||
return since.as_slice() <= &[rustc.major, rustc.minor, rustc.patch]; | ||
}; | ||
|
||
// Assume deprecation is in effect if "since" field is missing | ||
// or if we can't determine the current Rust version. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "or if we can't determine the current Rust version" part of this comment should have been removed by #117256. |
||
true | ||
} | ||
|
||
pub fn deprecation_suggestion( | ||
|
@@ -180,17 +160,15 @@ fn deprecation_lint(is_in_effect: bool) -> &'static Lint { | |
|
||
fn deprecation_message( | ||
is_in_effect: bool, | ||
since: Option<Symbol>, | ||
since: Option<DeprecatedSince>, | ||
note: Option<Symbol>, | ||
kind: &str, | ||
path: &str, | ||
) -> String { | ||
let message = if is_in_effect { | ||
format!("use of deprecated {kind} `{path}`") | ||
} else { | ||
let since = since.as_ref().map(Symbol::as_str); | ||
|
||
if since == Some("TBD") { | ||
if let Some(DeprecatedSince::Future) = since { | ||
format!("use of {kind} `{path}` that will be deprecated in a future Rust version") | ||
} else { | ||
format!( | ||
|
@@ -381,7 +359,7 @@ impl<'tcx> TyCtxt<'tcx> { | |
// With #![staged_api], we want to emit down the whole | ||
// hierarchy. | ||
let depr_attr = &depr_entry.attr; | ||
if !skip || depr_attr.is_since_rustc_version { | ||
if !skip || matches!(depr_attr.since, Some(DeprecatedSince::RustcVersion(_))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a is_since_rustc_version method? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's okay either way, I think. Here is what having a method looks like: 8b8906b. It's not that widely useful because most places that look for |
||
// Calculating message for lint involves calling `self.def_path_str`. | ||
// Which by default to calculate visible path will invoke expensive `visible_parent_map` query. | ||
// So we skip message calculation altogether, if lint is allowed. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a Missing variant and remove the Option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing
would make some things more complicated because then it no longer makes sense forDeprecatedSince
to implementDisplay
, which rustdoc currently relies on.Rustdoc may already need a different approach upon introducing an
Err
variant so I'll take a look.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After introducing
Err
, eliminating theOption
indeed seems like the right call. 👍