From 53fe37de2e11537bf2953a29df3c5be8a8858850 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Oct 2023 09:51:03 +1100 Subject: [PATCH 01/15] Remove unused `Span` from the `set` function in `State::Active`. --- compiler/rustc_expand/src/config.rs | 8 ++++---- compiler/rustc_feature/src/active.rs | 8 ++++---- compiler/rustc_feature/src/lib.rs | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 8658cea137a7d..b2546caa34574 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -23,7 +23,7 @@ use rustc_session::parse::feature_err; use rustc_session::Session; use rustc_span::edition::{Edition, ALL_EDITIONS}; use rustc_span::symbol::{sym, Symbol}; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::Span; /// A folder that strips out items that do not belong in the current configuration. pub struct StripUnconfigured<'a> { @@ -67,7 +67,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } for feature in active_features_up_to(crate_edition) { - feature.set(&mut features, DUMMY_SP); + feature.set(&mut features); edition_enabled_features.insert(feature.name, crate_edition); } @@ -98,7 +98,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { for feature in active_features_up_to(edition) { // FIXME(Manishearth) there is currently no way to set // lib features by edition - feature.set(&mut features, DUMMY_SP); + feature.set(&mut features); edition_enabled_features.insert(feature.name, edition); } } @@ -176,7 +176,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } if let Some(f) = ACTIVE_FEATURES.iter().find(|f| name == f.name) { - f.set(&mut features, mi.span()); + f.set(&mut features); features.declared_lang_features.push((name, mi.span(), None)); features.active_features.insert(name); continue; diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index a02c04ecd3ecb..5173dc7ccaa41 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -9,10 +9,10 @@ use rustc_span::Span; macro_rules! set { ($field: ident) => {{ - fn f(features: &mut Features, _: Span) { + fn f(features: &mut Features) { features.$field = true; } - f as fn(&mut Features, Span) + f as fn(&mut Features) }}; } @@ -123,9 +123,9 @@ macro_rules! declare_features { impl Feature { /// Sets this feature in `Features`. Panics if called on a non-active feature. - pub fn set(&self, features: &mut Features, span: Span) { + pub fn set(&self, features: &mut Features) { match self.state { - State::Active { set } => set(features, span), + State::Active { set } => set(features), _ => panic!("called `set` on feature `{}` which is not `active`", self.name), } } diff --git a/compiler/rustc_feature/src/lib.rs b/compiler/rustc_feature/src/lib.rs index 69e33115922ab..b4933d6339cc9 100644 --- a/compiler/rustc_feature/src/lib.rs +++ b/compiler/rustc_feature/src/lib.rs @@ -23,14 +23,14 @@ mod removed; #[cfg(test)] mod tests; -use rustc_span::{edition::Edition, symbol::Symbol, Span}; +use rustc_span::{edition::Edition, symbol::Symbol}; use std::fmt; use std::num::NonZeroU32; #[derive(Clone, Copy)] pub enum State { Accepted, - Active { set: fn(&mut Features, Span) }, + Active { set: fn(&mut Features) }, Removed { reason: Option<&'static str> }, Stabilized { reason: Option<&'static str> }, } From 043a9873b9ca6671744cf29c5cd0fe04c54e9032 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Oct 2023 10:06:17 +1100 Subject: [PATCH 02/15] Remove `set!` macro. It has a single call site. --- compiler/rustc_feature/src/active.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 5173dc7ccaa41..c5a2534789352 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -7,15 +7,6 @@ use rustc_span::edition::Edition; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; -macro_rules! set { - ($field: ident) => {{ - fn f(features: &mut Features) { - features.$field = true; - } - f as fn(&mut Features) - }}; -} - #[derive(PartialEq)] enum FeatureStatus { Default, @@ -43,7 +34,15 @@ macro_rules! declare_features { &[$( // (sym::$feature, $ver, $issue, $edition, set!($feature)) Feature { - state: State::Active { set: set!($feature) }, + state: State::Active { + // Sets this feature's corresponding bool within `features`. + set: { + fn f(features: &mut Features) { + features.$feature = true; + } + f as fn(&mut Features) + } + }, name: sym::$feature, since: $ver, issue: to_nonzero($issue), From 3c1b60c1b44b7a6e322982c5e19c0dc5e628680c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Oct 2023 10:00:30 +1100 Subject: [PATCH 03/15] Split `declare_features!`. It's a macro with four clauses, three of which are doing one thing, and the fourth is doing something completely different. This commit splits it into two macros, which is more sensible. --- compiler/rustc_feature/src/active.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index c5a2534789352..3acc7308e8dd8 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -14,16 +14,19 @@ enum FeatureStatus { Internal, } -macro_rules! declare_features { - (__status_to_enum active) => { +macro_rules! status_to_enum { + (active) => { FeatureStatus::Default }; - (__status_to_enum incomplete) => { + (incomplete) => { FeatureStatus::Incomplete }; - (__status_to_enum internal) => { + (internal) => { FeatureStatus::Internal }; +} + +macro_rules! declare_features { ($( $(#[doc = $doc:tt])* ($status:ident, $feature:ident, $ver:expr, $issue:expr, $edition:expr), )+) => { @@ -92,7 +95,7 @@ macro_rules! declare_features { pub fn incomplete(&self, feature: Symbol) -> bool { match feature { $( - sym::$feature => declare_features!(__status_to_enum $status) == FeatureStatus::Incomplete, + sym::$feature => status_to_enum!($status) == FeatureStatus::Incomplete, )* // accepted and removed features aren't in this file but are never incomplete _ if self.declared_lang_features.iter().any(|f| f.0 == feature) => false, @@ -107,7 +110,7 @@ macro_rules! declare_features { pub fn internal(&self, feature: Symbol) -> bool { match feature { $( - sym::$feature => declare_features!(__status_to_enum $status) == FeatureStatus::Internal, + sym::$feature => status_to_enum!($status) == FeatureStatus::Internal, )* // accepted and removed features aren't in this file but are never internal // (a removed feature might have been internal, but it doesn't matter anymore) From 1ddb2872ddeb5d56f7bdecfd4175fb13fc510069 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 4 Oct 2023 14:54:12 +1100 Subject: [PATCH 04/15] Streamline `find_lang_feature_issue`. It currently processes `ACTIVE_FEATURES` separately from `ACCEPTED_FEATURES`, `REMOVED_FEATURES`, and `STABLE_REMOVED_FEATURES`, for no good reason. This commit treats them uniformly. --- compiler/rustc_feature/src/lib.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_feature/src/lib.rs b/compiler/rustc_feature/src/lib.rs index b4933d6339cc9..d4dc5920beccd 100644 --- a/compiler/rustc_feature/src/lib.rs +++ b/compiler/rustc_feature/src/lib.rs @@ -79,8 +79,8 @@ pub enum UnstableFeatures { impl UnstableFeatures { /// This takes into account `RUSTC_BOOTSTRAP`. /// - /// If `krate` is [`Some`], then setting `RUSTC_BOOTSTRAP=krate` will enable the nightly features. - /// Otherwise, only `RUSTC_BOOTSTRAP=1` will work. + /// If `krate` is [`Some`], then setting `RUSTC_BOOTSTRAP=krate` will enable the nightly + /// features. Otherwise, only `RUSTC_BOOTSTRAP=1` will work. pub fn from_environment(krate: Option<&str>) -> Self { // `true` if this is a feature-staged build, i.e., on the beta or stable channel. let disable_unstable_features = @@ -107,19 +107,18 @@ impl UnstableFeatures { } fn find_lang_feature_issue(feature: Symbol) -> Option { - if let Some(info) = ACTIVE_FEATURES.iter().find(|t| t.name == feature) { - info.issue - } else { - // search in Accepted, Removed, or Stable Removed features - let found = ACCEPTED_FEATURES - .iter() - .chain(REMOVED_FEATURES) - .chain(STABLE_REMOVED_FEATURES) - .find(|t| t.name == feature); - match found { - Some(found) => found.issue, - None => panic!("feature `{feature}` is not declared anywhere"), - } + // Search in all the feature lists. + let found = [] + .iter() + .chain(ACTIVE_FEATURES) + .chain(ACCEPTED_FEATURES) + .chain(REMOVED_FEATURES) + .chain(STABLE_REMOVED_FEATURES) + .find(|t| t.name == feature); + + match found { + Some(found) => found.issue, + None => panic!("feature `{feature}` is not declared anywhere"), } } From e24f39440478dcddddb83b9999aa724d4a093d8f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 10:17:42 +1100 Subject: [PATCH 05/15] Add comments to `config::features`. I found this function very confusing, and it took me quite some time to work out what it was doing. These comments capture that hard-earned knowledge. --- compiler/rustc_expand/src/config.rs | 35 ++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index b2546caa34574..9cb16e0b0c91d 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -58,21 +58,28 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { let mut edition_enabled_features = FxHashMap::default(); let crate_edition = sess.edition(); + // Enable edition umbrella feature-gates based on the crate edition. + // - enable `rust_2015_preview` always + // - enable `rust_2018_preview` if the crate edition is 2018 or higher + // - enable `rust_2021_preview` if the crate edition is 2021 or higher + // - etc. for &edition in ALL_EDITIONS { if edition <= crate_edition { - // The `crate_edition` implies its respective umbrella feature-gate - // (i.e., `#![feature(rust_20XX_preview)]` isn't needed on edition 20XX). edition_enabled_features.insert(edition.feature_name(), edition); } } + // Enable edition-dependent features based on the crate edition. + // - E.g. `test_2018_feature` if the crate edition is 2018 or higher for feature in active_features_up_to(crate_edition) { feature.set(&mut features); edition_enabled_features.insert(feature.name, crate_edition); } - // Process the edition umbrella feature-gates first, to ensure - // `edition_enabled_features` is completed before it's queried. + // Enable edition umbrella feature-gates that are declared in the code. If + // present, enable edition-specific features based on that. + // - E.g. enable `test_2018_feature` if the crate edition is 2015 but + // `rust_2018_preview` is present for attr in krate_attrs { if !attr.has_name(sym::feature) { continue; @@ -105,6 +112,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } } + // Process all features declared in the code. for attr in krate_attrs { if !attr.has_name(sym::feature) { continue; @@ -136,6 +144,13 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } }; + // If the declared feature is edition-specific and already enabled + // due to the crate edition or a declared edition umbrella + // feature-gate, give a warning. + // - E.g. warn if `test_2018_feature` is declared when the crate + // edition is 2018 or higher + // - E.g. warn if `test_2018_feature` is declared when + // `rust_2018_preview` or higher is declared. if let Some(&edition) = edition_enabled_features.get(&name) { sess.emit_warning(FeatureIncludedInEdition { span: mi.span(), @@ -145,11 +160,14 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { continue; } + // If the declared feature is an edition umbrella feature-gate, + // ignore it, because it was already handled above. + // - E.g. `rust_20XX_preview` if ALL_EDITIONS.iter().any(|e| name == e.feature_name()) { - // Handled in the separate loop above. continue; } + // If the declared feature is removed, issue an error. let removed = REMOVED_FEATURES.iter().find(|f| name == f.name); let stable_removed = STABLE_REMOVED_FEATURES.iter().find(|f| name == f.name); if let Some(Feature { state, .. }) = removed.or(stable_removed) { @@ -161,6 +179,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } } + // If the declared feature is stable, record it. if let Some(Feature { since, .. }) = ACCEPTED_FEATURES.iter().find(|f| name == f.name) { let since = Some(Symbol::intern(since)); features.declared_lang_features.push((name, mi.span(), since)); @@ -168,6 +187,9 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { continue; } + // If `-Z allow-features` is used and the declared feature is + // unstable and not also listed as one of the allowed features, + // issue an error. if let Some(allowed) = sess.opts.unstable_opts.allow_features.as_ref() { if allowed.iter().all(|f| name.as_str() != f) { sess.emit_err(FeatureNotAllowed { span: mi.span(), name }); @@ -175,6 +197,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } } + // If the declared feature is unstable, record it. if let Some(f) = ACTIVE_FEATURES.iter().find(|f| name == f.name) { f.set(&mut features); features.declared_lang_features.push((name, mi.span(), None)); @@ -182,6 +205,8 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { continue; } + // Otherwise, the feature is unknown. Record it at a lib feature. + // It will be checked later. features.declared_lib_features.push((name, mi.span())); features.active_features.insert(name); } From 9e2cd038b03c8194a68e30dd823d145f1f0d4806 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 11:15:14 +1100 Subject: [PATCH 06/15] Factor out some repeated feature-getting code. --- compiler/rustc_expand/src/config.rs | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 9cb16e0b0c91d..0a7e424376d81 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -24,6 +24,7 @@ use rustc_session::Session; use rustc_span::edition::{Edition, ALL_EDITIONS}; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; +use thin_vec::ThinVec; /// A folder that strips out items that do not belong in the current configuration. pub struct StripUnconfigured<'a> { @@ -54,6 +55,14 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { }) } + fn feature_list(attr: &Attribute) -> ThinVec { + if attr.has_name(sym::feature) && let Some(list) = attr.meta_item_list() { + list + } else { + ThinVec::new() + } + } + let mut features = Features::default(); let mut edition_enabled_features = FxHashMap::default(); let crate_edition = sess.edition(); @@ -81,15 +90,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { // - E.g. enable `test_2018_feature` if the crate edition is 2015 but // `rust_2018_preview` is present for attr in krate_attrs { - if !attr.has_name(sym::feature) { - continue; - } - - let Some(list) = attr.meta_item_list() else { - continue; - }; - - for mi in list { + for mi in feature_list(attr) { if !mi.is_word() { continue; } @@ -114,15 +115,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { // Process all features declared in the code. for attr in krate_attrs { - if !attr.has_name(sym::feature) { - continue; - } - - let Some(list) = attr.meta_item_list() else { - continue; - }; - - for mi in list { + for mi in feature_list(attr) { let name = match mi.ident() { Some(ident) if mi.is_word() => ident.name, Some(ident) => { From 5d9559e0269d9d7db06c825d56d475757232c924 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 11:18:51 +1100 Subject: [PATCH 07/15] Inline and remove `feature_removed` function. It has a single call site. This increases consistency because other errors within `features` are emitted directly. --- compiler/rustc_expand/src/config.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 0a7e424376d81..e3d0af80c6a23 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -38,13 +38,6 @@ pub struct StripUnconfigured<'a> { } pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { - fn feature_removed(sess: &Session, span: Span, reason: Option<&str>) { - sess.emit_err(FeatureRemoved { - span, - reason: reason.map(|reason| FeatureRemovedReason { reason }), - }); - } - fn active_features_up_to(edition: Edition) -> impl Iterator { ACTIVE_FEATURES.iter().filter(move |feature| { if let Some(feature_edition) = feature.edition { @@ -167,7 +160,10 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { if let FeatureState::Removed { reason } | FeatureState::Stabilized { reason } = state { - feature_removed(sess, mi.span(), *reason); + sess.emit_err(FeatureRemoved { + span: mi.span(), + reason: reason.map(|reason| FeatureRemovedReason { reason }), + }); continue; } } From 8ba91378401f5483b1d834be33911028507a9d5d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 11:38:43 +1100 Subject: [PATCH 08/15] Merge `STABLE_REMOVED_FEATURES` list into `REMOVED_FEATURES`. There is a single features (`no_stack_check`) in `STABLE_REMOVED_FEATURES`. But the treatment of `STABLE_REMOVED_FEATURES` and `REMOVED_FEATURES` is actually identical. So this commit just merges them, and uses a comment to record `no_stack_check`'s unique "stable removed" status. This also lets `State::Stabilized` (which was a terrible name) be removed. --- compiler/rustc_expand/src/config.rs | 12 +++--------- compiler/rustc_feature/src/lib.rs | 5 +---- compiler/rustc_feature/src/removed.rs | 27 +++++---------------------- 3 files changed, 9 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index e3d0af80c6a23..d3f792601b797 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -15,9 +15,7 @@ use rustc_attr as attr; use rustc_data_structures::flat_map_in_place::FlatMapInPlace; use rustc_data_structures::fx::FxHashMap; use rustc_feature::{Feature, Features, State as FeatureState}; -use rustc_feature::{ - ACCEPTED_FEATURES, ACTIVE_FEATURES, REMOVED_FEATURES, STABLE_REMOVED_FEATURES, -}; +use rustc_feature::{ACCEPTED_FEATURES, ACTIVE_FEATURES, REMOVED_FEATURES}; use rustc_parse::validate_attr; use rustc_session::parse::feature_err; use rustc_session::Session; @@ -154,12 +152,8 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } // If the declared feature is removed, issue an error. - let removed = REMOVED_FEATURES.iter().find(|f| name == f.name); - let stable_removed = STABLE_REMOVED_FEATURES.iter().find(|f| name == f.name); - if let Some(Feature { state, .. }) = removed.or(stable_removed) { - if let FeatureState::Removed { reason } | FeatureState::Stabilized { reason } = - state - { + if let Some(Feature { state, .. }) = REMOVED_FEATURES.iter().find(|f| name == f.name) { + if let FeatureState::Removed { reason } = state { sess.emit_err(FeatureRemoved { span: mi.span(), reason: reason.map(|reason| FeatureRemovedReason { reason }), diff --git a/compiler/rustc_feature/src/lib.rs b/compiler/rustc_feature/src/lib.rs index d4dc5920beccd..4721bff0ec719 100644 --- a/compiler/rustc_feature/src/lib.rs +++ b/compiler/rustc_feature/src/lib.rs @@ -32,7 +32,6 @@ pub enum State { Accepted, Active { set: fn(&mut Features) }, Removed { reason: Option<&'static str> }, - Stabilized { reason: Option<&'static str> }, } impl fmt::Debug for State { @@ -41,7 +40,6 @@ impl fmt::Debug for State { State::Accepted { .. } => write!(f, "accepted"), State::Active { .. } => write!(f, "active"), State::Removed { .. } => write!(f, "removed"), - State::Stabilized { .. } => write!(f, "stabilized"), } } } @@ -113,7 +111,6 @@ fn find_lang_feature_issue(feature: Symbol) -> Option { .chain(ACTIVE_FEATURES) .chain(ACCEPTED_FEATURES) .chain(REMOVED_FEATURES) - .chain(STABLE_REMOVED_FEATURES) .find(|t| t.name == feature); match found { @@ -151,4 +148,4 @@ pub use builtin_attrs::{ is_valid_for_get_attr, AttributeGate, AttributeTemplate, AttributeType, BuiltinAttribute, GatedCfg, BUILTIN_ATTRIBUTES, BUILTIN_ATTRIBUTE_MAP, }; -pub use removed::{REMOVED_FEATURES, STABLE_REMOVED_FEATURES}; +pub use removed::REMOVED_FEATURES; diff --git a/compiler/rustc_feature/src/removed.rs b/compiler/rustc_feature/src/removed.rs index da18cb2a239e8..de15deef1789e 100644 --- a/compiler/rustc_feature/src/removed.rs +++ b/compiler/rustc_feature/src/removed.rs @@ -20,23 +20,6 @@ macro_rules! declare_features { ),+ ]; }; - - ($( - $(#[doc = $doc:tt])* (stable_removed, $feature:ident, $ver:expr, $issue:expr, None), - )+) => { - /// Represents stable features which have since been removed (it was once Accepted) - pub const STABLE_REMOVED_FEATURES: &[Feature] = &[ - $( - Feature { - state: State::Stabilized { reason: None }, - name: sym::$feature, - since: $ver, - issue: to_nonzero($issue), - edition: None, - } - ),+ - ]; - }; } #[rustfmt::skip] @@ -141,6 +124,11 @@ declare_features! ( (removed, no_coverage, "CURRENT_RUSTC_VERSION", Some(84605), None, Some("renamed to `coverage_attribute`")), /// Allows `#[no_debug]`. (removed, no_debug, "1.43.0", Some(29721), None, Some("removed due to lack of demand")), + /// Note: this feature was previously recorded in a separate + /// `STABLE_REMOVED` list because it, uniquely, was once stable but was + /// then removed. But there was no utility storing it separately, so now + /// it's in this list. + (removed, no_stack_check, "1.0.0", None, None, None), /// Allows using `#[on_unimplemented(..)]` on traits. /// (Moved to `rustc_attrs`.) (removed, on_unimplemented, "1.40.0", None, None, None), @@ -208,8 +196,3 @@ declare_features! ( // feature-group-end: removed features // ------------------------------------------------------------------------- ); - -#[rustfmt::skip] -declare_features! ( - (stable_removed, no_stack_check, "1.0.0", None, None), -); From b229be0127dacde928826f6dac697230c74eef05 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 12:38:02 +1100 Subject: [PATCH 09/15] Overhaul `config::features`. The new way of doing things: - Avoids some code duplication. - Distinguishes the `crate_edition` (which comes from `--edition`) and the `features_edition` (which combines `--edition` along with any `rustc_20XX_preview` features), which is useful. - Has a simpler initial loop, one that just looks for `rustc_20XX_preview` features in order to compute `features_edition`. - Creates a fallible alternative to `Features::enabled`, which is useful. It's not easy to see how exactly the old and new code are equivalent, but it's reassuring to know that the test coverage is quite good for this stuff. --- compiler/rustc_expand/src/config.rs | 103 +++++++++++++--------------- 1 file changed, 46 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index d3f792601b797..b3ee755f565df 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -13,7 +13,7 @@ use rustc_ast::NodeId; use rustc_ast::{self as ast, AttrStyle, Attribute, HasAttrs, HasTokens, MetaItem}; use rustc_attr as attr; use rustc_data_structures::flat_map_in_place::FlatMapInPlace; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::FxHashSet; use rustc_feature::{Feature, Features, State as FeatureState}; use rustc_feature::{ACCEPTED_FEATURES, ACTIVE_FEATURES, REMOVED_FEATURES}; use rustc_parse::validate_attr; @@ -55,55 +55,37 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } let mut features = Features::default(); - let mut edition_enabled_features = FxHashMap::default(); - let crate_edition = sess.edition(); - - // Enable edition umbrella feature-gates based on the crate edition. - // - enable `rust_2015_preview` always - // - enable `rust_2018_preview` if the crate edition is 2018 or higher - // - enable `rust_2021_preview` if the crate edition is 2021 or higher - // - etc. - for &edition in ALL_EDITIONS { - if edition <= crate_edition { - edition_enabled_features.insert(edition.feature_name(), edition); - } - } - // Enable edition-dependent features based on the crate edition. - // - E.g. `test_2018_feature` if the crate edition is 2018 or higher - for feature in active_features_up_to(crate_edition) { - feature.set(&mut features); - edition_enabled_features.insert(feature.name, crate_edition); - } + // The edition from `--edition`. + let crate_edition = sess.edition(); - // Enable edition umbrella feature-gates that are declared in the code. If - // present, enable edition-specific features based on that. - // - E.g. enable `test_2018_feature` if the crate edition is 2015 but - // `rust_2018_preview` is present + // The maximum of (a) the edition from `--edition` and (b) any edition + // umbrella feature-gates declared in the code. + // - E.g. if `crate_edition` is 2015 but `rust_2018_preview` is present, + // `feature_edition` is 2018 + let mut features_edition = crate_edition; for attr in krate_attrs { for mi in feature_list(attr) { - if !mi.is_word() { - continue; - } - - let name = mi.name_or_empty(); - - let edition = ALL_EDITIONS.iter().find(|e| name == e.feature_name()).copied(); - if let Some(edition) = edition { - if edition <= crate_edition { - continue; - } - - for feature in active_features_up_to(edition) { - // FIXME(Manishearth) there is currently no way to set - // lib features by edition - feature.set(&mut features); - edition_enabled_features.insert(feature.name, edition); + if mi.is_word() { + let name = mi.name_or_empty(); + let edition = ALL_EDITIONS.iter().find(|e| name == e.feature_name()).copied(); + if let Some(edition) = edition && edition > features_edition { + features_edition = edition; } } } } + // Enable edition-dependent features based on `features_edition`. + // - E.g. enable `test_2018_feature` if `features_edition` is 2018 or higher + let mut edition_enabled_features = FxHashSet::default(); + for feature in active_features_up_to(features_edition) { + // FIXME(Manishearth) there is currently no way to set lib features by + // edition. + edition_enabled_features.insert(feature.name); + feature.set(&mut features); + } + // Process all features declared in the code. for attr in krate_attrs { for mi in feature_list(attr) { @@ -128,30 +110,37 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { } }; - // If the declared feature is edition-specific and already enabled - // due to the crate edition or a declared edition umbrella - // feature-gate, give a warning. - // - E.g. warn if `test_2018_feature` is declared when the crate - // edition is 2018 or higher + // If the declared feature is an edition umbrella feature-gate, + // warn if it was redundant w.r.t. `crate_edition`. + // - E.g. warn if `rust_2018_preview` is declared when + // `crate_edition` is 2018 + // - E.g. don't warn if `rust_2018_preview` is declared when + // `crate_edition` is 2015. + if let Some(&edition) = ALL_EDITIONS.iter().find(|e| name == e.feature_name()) { + if edition <= crate_edition { + sess.emit_warning(FeatureIncludedInEdition { + span: mi.span(), + feature: name, + edition, + }); + } + continue; + } + + // If the declared feature is edition-dependent and was already + // enabled due to `feature_edition`, give a warning. // - E.g. warn if `test_2018_feature` is declared when - // `rust_2018_preview` or higher is declared. - if let Some(&edition) = edition_enabled_features.get(&name) { + // `feature_edition` is 2018 or higher. + if edition_enabled_features.contains(&name) { sess.emit_warning(FeatureIncludedInEdition { span: mi.span(), feature: name, - edition, + edition: features_edition, }); continue; } - // If the declared feature is an edition umbrella feature-gate, - // ignore it, because it was already handled above. - // - E.g. `rust_20XX_preview` - if ALL_EDITIONS.iter().any(|e| name == e.feature_name()) { - continue; - } - - // If the declared feature is removed, issue an error. + // If the declared feature has been removed, issue an error. if let Some(Feature { state, .. }) = REMOVED_FEATURES.iter().find(|f| name == f.name) { if let FeatureState::Removed { reason } = state { sess.emit_err(FeatureRemoved { From 4602d9257de0939ff8770dcf7f10ac1832bce35f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 16:08:07 +1100 Subject: [PATCH 10/15] Rename `Features::active_features`. The word "active" is currently used in two different and confusing ways: - `ACTIVE_FEATURES` actually means "available unstable features" - `Features::active_features` actually means "features declared in the crate's code", which can include feature within `ACTIVE_FEATURES` but also others. (This is also distinct from "enabled" features which includes declared features but also some edition-specific features automatically enabled depending on the edition in use.) This commit changes the `Features::active_features` to `Features::declared_features` which actually matches its meaning. Likewise, `Features::active` becomes `Features::declared`. --- compiler/rustc_expand/src/config.rs | 6 +++--- compiler/rustc_feature/src/active.rs | 15 +++++++++------ compiler/rustc_middle/src/middle/stability.rs | 6 +++--- .../clippy_lints/src/manual_float_methods.rs | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index b3ee755f565df..ed619e02f57fa 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -155,7 +155,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { if let Some(Feature { since, .. }) = ACCEPTED_FEATURES.iter().find(|f| name == f.name) { let since = Some(Symbol::intern(since)); features.declared_lang_features.push((name, mi.span(), since)); - features.active_features.insert(name); + features.declared_features.insert(name); continue; } @@ -173,14 +173,14 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { if let Some(f) = ACTIVE_FEATURES.iter().find(|f| name == f.name) { f.set(&mut features); features.declared_lang_features.push((name, mi.span(), None)); - features.active_features.insert(name); + features.declared_features.insert(name); continue; } // Otherwise, the feature is unknown. Record it at a lib feature. // It will be checked later. features.declared_lib_features.push((name, mi.span())); - features.active_features.insert(name); + features.declared_features.insert(name); } } diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 3acc7308e8dd8..07101778ef1d4 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -60,8 +60,9 @@ macro_rules! declare_features { pub declared_lang_features: Vec<(Symbol, Span, Option)>, /// `#![feature]` attrs for non-language (library) features. pub declared_lib_features: Vec<(Symbol, Span)>, - /// Features enabled for this crate. - pub active_features: FxHashSet, + /// `declared_lang_features` + `declared_lib_features`. + pub declared_features: FxHashSet, + /// Individual features (unstable only). $( $(#[doc = $doc])* pub $feature: bool @@ -73,12 +74,14 @@ macro_rules! declare_features { $(f(stringify!($feature), self.$feature);)+ } - /// Is the given feature active? - pub fn active(&self, feature: Symbol) -> bool { - self.active_features.contains(&feature) + /// Is the given feature explicitly declared, i.e. named in a + /// `#![feature(...)]` within the code? + pub fn declared(&self, feature: Symbol) -> bool { + self.declared_features.contains(&feature) } - /// Is the given feature enabled? + /// Is the given feature enabled, i.e. declared or automatically + /// enabled due to the edition? /// /// Panics if the symbol doesn't correspond to a declared feature. pub fn enabled(&self, feature: Symbol) -> bool { diff --git a/compiler/rustc_middle/src/middle/stability.rs b/compiler/rustc_middle/src/middle/stability.rs index 908ab8b613e88..c66f64dde3291 100644 --- a/compiler/rustc_middle/src/middle/stability.rs +++ b/compiler/rustc_middle/src/middle/stability.rs @@ -448,14 +448,14 @@ impl<'tcx> TyCtxt<'tcx> { debug!("stability: skipping span={:?} since it is internal", span); return EvalResult::Allow; } - if self.features().active(feature) { + if self.features().declared(feature) { return EvalResult::Allow; } // If this item was previously part of a now-stabilized feature which is still // active (i.e. the user hasn't removed the attribute for the stabilized feature // yet) then allow use of this item. - if let Some(implied_by) = implied_by && self.features().active(implied_by) { + if let Some(implied_by) = implied_by && self.features().declared(implied_by) { return EvalResult::Allow; } @@ -532,7 +532,7 @@ impl<'tcx> TyCtxt<'tcx> { debug!("body stability: skipping span={:?} since it is internal", span); return EvalResult::Allow; } - if self.features().active(feature) { + if self.features().declared(feature) { return EvalResult::Allow; } diff --git a/src/tools/clippy/clippy_lints/src/manual_float_methods.rs b/src/tools/clippy/clippy_lints/src/manual_float_methods.rs index 88db7ae6aece0..15c8417e70284 100644 --- a/src/tools/clippy/clippy_lints/src/manual_float_methods.rs +++ b/src/tools/clippy/clippy_lints/src/manual_float_methods.rs @@ -85,7 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualFloatMethods { if !in_external_macro(cx.sess(), expr.span) && ( matches!(cx.tcx.constness(cx.tcx.hir().enclosing_body_owner(expr.hir_id)), Constness::NotConst) - || cx.tcx.features().active(sym!(const_float_classify)) + || cx.tcx.features().declared(sym!(const_float_classify)) ) && let ExprKind::Binary(kind, lhs, rhs) = expr.kind && let ExprKind::Binary(lhs_kind, lhs_lhs, lhs_rhs) = lhs.kind && let ExprKind::Binary(rhs_kind, rhs_lhs, rhs_rhs) = rhs.kind From eb209057b1c7a1e7852793e8f53eadfc9d839b2c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 16:20:07 +1100 Subject: [PATCH 11/15] Rename `Resolver::active_features`. For the reasons described in the previous commit. --- compiler/rustc_resolve/src/lib.rs | 11 +++-------- compiler/rustc_resolve/src/macros.rs | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 949c6ab5ac0c1..5f012ec29fe00 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1074,8 +1074,8 @@ pub struct Resolver<'a, 'tcx> { /// Also includes of list of each fields visibility struct_constructors: LocalDefIdMap<(Res, ty::Visibility, Vec>)>, - /// Features enabled for this crate. - active_features: FxHashSet, + /// Features declared for this crate. + declared_features: FxHashSet, lint_buffer: LintBuffer, @@ -1417,12 +1417,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { multi_segment_macro_resolutions: Default::default(), builtin_attrs: Default::default(), containers_deriving_copy: Default::default(), - active_features: features - .declared_lib_features - .iter() - .map(|(feat, ..)| *feat) - .chain(features.declared_lang_features.iter().map(|(feat, ..)| *feat)) - .collect(), + declared_features: features.declared_features.clone(), lint_buffer: LintBuffer::default(), next_node_id: CRATE_NODE_ID, node_id_to_def_id, diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 90ae08ce37c1f..f0a1a4ff931e9 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -854,7 +854,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let feature = stability.feature; let is_allowed = |feature| { - self.active_features.contains(&feature) || span.allows_unstable(feature) + self.declared_features.contains(&feature) || span.allows_unstable(feature) }; let allowed_by_implication = implied_by.is_some_and(|feature| is_allowed(feature)); if !is_allowed(feature) && !allowed_by_implication { From 95d1aa075f1bdd7acf1b0c0f0b918b3371cf987d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 16:24:31 +1100 Subject: [PATCH 12/15] Record all declared features. Currently `rust_20XX_preview` features aren't recorded as declared even when they are explicit declared. Similarly, redundant edition-dependent features (e.g. `test_2018_feature`) aren't recorded as declared. This commit marks them as recorded. There is no detectable functional change, but it makes things more consistent. --- compiler/rustc_expand/src/config.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index ed619e02f57fa..40a231c4e8012 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -124,6 +124,8 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { edition, }); } + features.declared_lang_features.push((name, mi.span(), None)); + features.declared_features.insert(name); continue; } @@ -137,6 +139,8 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { feature: name, edition: features_edition, }); + features.declared_lang_features.push((name, mi.span(), None)); + features.declared_features.insert(name); continue; } From 56fd2531ac44f95295f78b9f55383bfebc26d358 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 16:33:42 +1100 Subject: [PATCH 13/15] Add two setter functions to `Features`. --- compiler/rustc_expand/src/config.rs | 17 ++++++----------- compiler/rustc_feature/src/active.rs | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 40a231c4e8012..7ad0e799f4464 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -124,8 +124,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { edition, }); } - features.declared_lang_features.push((name, mi.span(), None)); - features.declared_features.insert(name); + features.set_declared_lang_feature(name, mi.span(), None); continue; } @@ -139,8 +138,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { feature: name, edition: features_edition, }); - features.declared_lang_features.push((name, mi.span(), None)); - features.declared_features.insert(name); + features.set_declared_lang_feature(name, mi.span(), None); continue; } @@ -158,8 +156,7 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { // If the declared feature is stable, record it. if let Some(Feature { since, .. }) = ACCEPTED_FEATURES.iter().find(|f| name == f.name) { let since = Some(Symbol::intern(since)); - features.declared_lang_features.push((name, mi.span(), since)); - features.declared_features.insert(name); + features.set_declared_lang_feature(name, mi.span(), since); continue; } @@ -176,15 +173,13 @@ pub fn features(sess: &Session, krate_attrs: &[Attribute]) -> Features { // If the declared feature is unstable, record it. if let Some(f) = ACTIVE_FEATURES.iter().find(|f| name == f.name) { f.set(&mut features); - features.declared_lang_features.push((name, mi.span(), None)); - features.declared_features.insert(name); + features.set_declared_lang_feature(name, mi.span(), None); continue; } - // Otherwise, the feature is unknown. Record it at a lib feature. + // Otherwise, the feature is unknown. Record it as a lib feature. // It will be checked later. - features.declared_lib_features.push((name, mi.span())); - features.declared_features.insert(name); + features.set_declared_lib_feature(name, mi.span()); } } diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 07101778ef1d4..75cb84d1b56fb 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -70,6 +70,21 @@ macro_rules! declare_features { } impl Features { + pub fn set_declared_lang_feature( + &mut self, + symbol: Symbol, + span: Span, + since: Option + ) { + self.declared_lang_features.push((symbol, span, since)); + self.declared_features.insert(symbol); + } + + pub fn set_declared_lib_feature(&mut self, symbol: Symbol, span: Span) { + self.declared_lib_features.push((symbol, span)); + self.declared_features.insert(symbol); + } + pub fn walk_feature_fields(&self, mut f: impl FnMut(&str, bool)) { $(f(stringify!($feature), self.$feature);)+ } From 9d4e49b386c66520eb6434405a86ef56614d84bc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 16:35:50 +1100 Subject: [PATCH 14/15] Use `declared_features` to avoid two lookups. --- compiler/rustc_feature/src/active.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 75cb84d1b56fb..abfaef8b0ff23 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -116,8 +116,7 @@ macro_rules! declare_features { sym::$feature => status_to_enum!($status) == FeatureStatus::Incomplete, )* // accepted and removed features aren't in this file but are never incomplete - _ if self.declared_lang_features.iter().any(|f| f.0 == feature) => false, - _ if self.declared_lib_features.iter().any(|f| f.0 == feature) => false, + _ if self.declared_features.contains(&feature) => false, _ => panic!("`{}` was not listed in `declare_features`", feature), } } @@ -132,8 +131,7 @@ macro_rules! declare_features { )* // accepted and removed features aren't in this file but are never internal // (a removed feature might have been internal, but it doesn't matter anymore) - _ if self.declared_lang_features.iter().any(|f| f.0 == feature) => false, - _ if self.declared_lib_features.iter().any(|f| f.0 == feature) => false, + _ if self.declared_features.contains(&feature) => false, _ => panic!("`{}` was not listed in `declare_features`", feature), } } From 81d1f7ea9d86490de2920d7b726db412c21de005 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 5 Oct 2023 19:56:19 +1100 Subject: [PATCH 15/15] Use a closure when setting `State::Active`. --- compiler/rustc_feature/src/active.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index abfaef8b0ff23..83961647bd442 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -39,12 +39,7 @@ macro_rules! declare_features { Feature { state: State::Active { // Sets this feature's corresponding bool within `features`. - set: { - fn f(features: &mut Features) { - features.$feature = true; - } - f as fn(&mut Features) - } + set: |features| features.$feature = true, }, name: sym::$feature, since: $ver,