Auto merge of #66945 - Centril:de-macro-gating, r=<try>

de-macro-ize feature gate checking

This PR removes the usage of macros in feature gating preferring to take in symbols instead. The aim is to make feature gating more understandable overall while also reducing some duplication.

To check whether a feature gate is active, `tcx.features().on(sym::my_feature)` can be used.
Inside `PostExpansionVisitor` it's however better to use `self.gate(...)` which provides a nicer API atop of that. Outside of the visitor, if `gate_feature` can be used, it should be preferred over `feature_err`.

As a follow-up, and once #66878 is merged, it might be a good idea to add a method to `Session` for more convenient access to `gate_feature` and `feature_err` respectively.

Originally I intended to go for a `HashSet` representation, but this felt like a smaller change to just expand to a `match` on the symbol => field pairs.

r? @oli-obk

Author: bors

src/librustc/hir/lowering.rs

@@ -834,7 +834,7 @@ impl<'a> LoweringContext<'a> {
             return;
         }
 
-        if !self.sess.features_untracked().in_band_lifetimes {
+        if !self.sess.features_untracked().on(sym::in_band_lifetimes) {
             return;
         }
 
@@ -1394,7 +1394,7 @@ impl<'a> LoweringContext<'a> {
                     }
                     ImplTraitContext::Disallowed(pos) => {
                         let allowed_in = if self.sess.features_untracked()
-                                                .impl_trait_in_bindings {
+                                                .on(sym::impl_trait_in_bindings) {
                             "bindings or function and inherent method return types"
                         } else {
                             "function and inherent method return types"
@@ -2118,7 +2118,7 @@ impl<'a> LoweringContext<'a> {
 
     fn lower_local(&mut self, l: &Local) -> (hir::Local, SmallVec<[NodeId; 1]>) {
         let mut ids = SmallVec::<[NodeId; 1]>::new();
-        if self.sess.features_untracked().impl_trait_in_bindings {
+        if self.sess.features_untracked().on(sym::impl_trait_in_bindings) {
             if let Some(ref ty) = l.ty {
                 let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
                 visitor.visit_ty(ty);
@@ -2130,7 +2130,7 @@ impl<'a> LoweringContext<'a> {
             ty: l.ty
                 .as_ref()
                 .map(|t| self.lower_ty(t,
-                    if self.sess.features_untracked().impl_trait_in_bindings {
+                    if self.sess.features_untracked().on(sym::impl_trait_in_bindings) {
                         ImplTraitContext::OpaqueTy(Some(parent_def_id))
                     } else {
                         ImplTraitContext::Disallowed(ImplTraitPosition::Binding)

src/librustc/hir/lowering/item.rs

@@ -181,15 +181,15 @@ impl LoweringContext<'_> {
             ItemKind::Impl(.., None, _, _) => smallvec![i.id],
             ItemKind::Static(ref ty, ..) => {
                 let mut ids = smallvec![i.id];
-                if self.sess.features_untracked().impl_trait_in_bindings {
+                if self.sess.features_untracked().on(sym::impl_trait_in_bindings) {
                     let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
                     visitor.visit_ty(ty);
                 }
                 ids
             },
             ItemKind::Const(ref ty, ..) => {
                 let mut ids = smallvec![i.id];
-                if self.sess.features_untracked().impl_trait_in_bindings {
+                if self.sess.features_untracked().on(sym::impl_trait_in_bindings) {
                     let mut visitor = ImplTraitTypeIdVisitor { ids: &mut ids };
                     visitor.visit_ty(ty);
                 }
@@ -285,7 +285,7 @@ impl LoweringContext<'_> {
                 hir::ItemKind::Static(
                     self.lower_ty(
                         t,
-                        if self.sess.features_untracked().impl_trait_in_bindings {
+                        if self.sess.features_untracked().on(sym::impl_trait_in_bindings) {
                             ImplTraitContext::OpaqueTy(None)
                         } else {
                             ImplTraitContext::Disallowed(ImplTraitPosition::Binding)
@@ -299,7 +299,7 @@ impl LoweringContext<'_> {
                 hir::ItemKind::Const(
                     self.lower_ty(
                         t,
-                        if self.sess.features_untracked().impl_trait_in_bindings {
+                        if self.sess.features_untracked().on(sym::impl_trait_in_bindings) {
                             ImplTraitContext::OpaqueTy(None)
                         } else {
                             ImplTraitContext::Disallowed(ImplTraitPosition::Binding)

src/librustc/infer/error_reporting/need_type_info.rs

@@ -5,8 +5,8 @@ use crate::infer::InferCtxt;
 use crate::infer::type_variable::TypeVariableOriginKind;
 use crate::ty::{self, Ty, Infer, TyVar};
 use crate::ty::print::Print;
-use syntax::source_map::DesugaringKind;
-use syntax_pos::Span;
+use syntax_pos::source_map::DesugaringKind;
+use syntax_pos::{Span, symbol::sym};
 use errors::{Applicability, DiagnosticBuilder};
 
 use rustc_error_codes::*;
@@ -217,7 +217,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         let is_named_and_not_impl_trait = |ty: Ty<'_>| {
             &ty.to_string() != "_" &&
                 // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
-                (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings)
+                (!ty.is_impl_trait() || self.tcx.features().on(sym::impl_trait_in_bindings))
         };
 
         let ty_msg = match local_visitor.found_ty {

src/librustc/infer/opaque_types/mod.rs

@@ -13,7 +13,7 @@ use errors::DiagnosticBuilder;
 use rustc::session::config::nightly_options;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::sync::Lrc;
-use syntax_pos::Span;
+use syntax_pos::{Span, symbol::sym};
 
 use rustc_error_codes::*;
 
@@ -488,7 +488,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         conflict2: ty::Region<'tcx>,
     ) -> bool {
         // If we have `#![feature(member_constraints)]`, no problems.
-        if self.tcx.features().member_constraints {
+        if self.tcx.features().on(sym::member_constraints) {
             return false;
         }
 

src/librustc/lint/levels.rs

@@ -231,15 +231,13 @@ impl<'a> LintLevelsBuilder<'a> {
                             // FIXME (#55112): issue unused-attributes lint if we thereby
                             // don't have any lint names (`#[level(reason = "foo")]`)
                             if let ast::LitKind::Str(rationale, _) = name_value.kind {
-                                if !self.sess.features_untracked().lint_reasons {
-                                    feature_gate::feature_err(
-                                        &self.sess.parse_sess,
-                                        sym::lint_reasons,
-                                        item.span,
-                                        "lint reasons are experimental"
-                                    )
-                                    .emit();
-                                }
+                                feature_gate::gate_feature(
+                                    &self.sess.parse_sess,
+                                    self.sess.features_untracked(),
+                                    item.span,
+                                    sym::lint_reasons,
+                                    "lint reasons are experimental"
+                                );
                                 reason = Some(rationale);
                             } else {
                                 bad_attr(name_value.span)

src/librustc/middle/stability.rs

@@ -116,7 +116,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
                    item_sp: Span, kind: AnnotationKind, visit_children: F)
         where F: FnOnce(&mut Self)
     {
-        if self.tcx.features().staged_api {
+        if self.tcx.features().on(sym::staged_api) {
             // This crate explicitly wants staged API.
             debug!("annotate(id = {:?}, attrs = {:?})", hir_id, attrs);
             if let Some(..) = attr::find_deprecation(&self.tcx.sess.parse_sess, attrs, item_sp) {
@@ -393,7 +393,7 @@ impl<'tcx> Index<'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>) -> Index<'tcx> {
         let is_staged_api =
             tcx.sess.opts.debugging_opts.force_unstable_if_unmarked ||
-            tcx.features().staged_api;
+            tcx.features().on(sym::staged_api);
         let mut staged_api = FxHashMap::default();
         staged_api.insert(LOCAL_CRATE, is_staged_api);
         let mut index = Index {
@@ -836,7 +836,7 @@ impl Visitor<'tcx> for Checker<'tcx> {
 
             // There's no good place to insert stability check for non-Copy unions,
             // so semi-randomly perform it here in stability.rs
-            hir::ItemKind::Union(..) if !self.tcx.features().untagged_unions => {
+            hir::ItemKind::Union(..) if !self.tcx.features().on(sym::untagged_unions) => {
                 let def_id = self.tcx.hir().local_def_id(item.hir_id);
                 let adt_def = self.tcx.adt_def(def_id);
                 let ty = self.tcx.type_of(def_id);

src/librustc/traits/error_reporting.rs

@@ -2353,13 +2353,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             }
             ObligationCauseCode::VariableType(_) => {
                 err.note("all local variables must have a statically known size");
-                if !self.tcx.features().unsized_locals {
+                if !self.tcx.features().on(sym::unsized_locals) {
                     err.help("unsized locals are gated as an unstable feature");
                 }
             }
             ObligationCauseCode::SizedArgumentType => {
                 err.note("all function arguments must have a statically known size");
-                if !self.tcx.features().unsized_locals {
+                if !self.tcx.features().on(sym::unsized_locals) {
                     err.help("unsized locals are gated as an unstable feature");
                 }
             }

src/librustc/traits/select.rs

@@ -2215,7 +2215,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     }
 
                     if let Some(principal) = data.principal() {
-                        if !self.infcx.tcx.features().object_safe_for_dispatch {
+                        if !self.infcx.tcx.features().on(sym::object_safe_for_dispatch) {
                             principal.with_self_ty(self.tcx(), self_ty)
                         } else if self.tcx().is_object_safe(principal.def_id()) {
                             principal.with_self_ty(self.tcx(), self_ty)

src/librustc/traits/specialize/mod.rs

@@ -16,7 +16,7 @@ use crate::infer::{InferCtxt, InferOk};
 use crate::lint;
 use crate::traits::{self, coherence, FutureCompatOverlapErrorKind, ObligationCause, TraitEngine};
 use rustc_data_structures::fx::FxHashSet;
-use syntax_pos::DUMMY_SP;
+use syntax_pos::{DUMMY_SP, symbol::sym};
 use crate::traits::select::IntercrateAmbiguityCause;
 use crate::ty::{self, TyCtxt, TypeFoldable};
 use crate::ty::subst::{Subst, InternalSubsts, SubstsRef};
@@ -155,7 +155,7 @@ pub(super) fn specializes(
 
     // The feature gate should prevent introducing new specializations, but not
     // taking advantage of upstream ones.
-    if !tcx.features().specialization &&
+    if !tcx.features().on(sym::specialization) &&
         (impl1_def_id.is_local() || impl2_def_id.is_local()) {
         return false;
     }

src/librustc/ty/constness.rs

@@ -2,7 +2,7 @@ use crate::ty::query::Providers;
 use crate::hir::def_id::DefId;
 use crate::hir;
 use crate::ty::TyCtxt;
-use syntax_pos::symbol::Symbol;
+use syntax_pos::symbol::{sym, Symbol};
 use crate::hir::map::blocks::FnLikeNode;
 use syntax::attr;
 
@@ -42,7 +42,7 @@ impl<'tcx> TyCtxt<'tcx> {
             return false;
         }
 
-        if self.features().staged_api {
+        if self.features().on(sym::staged_api) {
             // in order for a libstd function to be considered min_const_fn
             // it needs to be stable and have no `rustc_const_unstable` attribute
             match self.lookup_stability(def_id) {
@@ -56,7 +56,7 @@ impl<'tcx> TyCtxt<'tcx> {
             }
         } else {
             // users enabling the `const_fn` feature gate can do what they want
-            !self.features().const_fn
+            !self.features().on(sym::const_fn)
         }
     }
 }

src/librustc/ty/context.rs

@@ -1473,7 +1473,7 @@ impl<'tcx> TyCtxt<'tcx> {
         //
         // * Otherwise, use the behavior requested via `-Z borrowck=...`
 
-        if self.features().nll { return BorrowckMode::Mir; }
+        if self.features().on(sym::nll) { return BorrowckMode::Mir; }
 
         self.sess.opts.borrowck_mode
     }
@@ -2437,7 +2437,7 @@ impl<'tcx> TyCtxt<'tcx> {
 
     #[inline]
     pub fn mk_diverging_default(self) -> Ty<'tcx> {
-        if self.features().never_type_fallback {
+        if self.features().on(sym::never_type_fallback) {
             self.types.never
         } else {
             self.types.unit

src/librustc/ty/mod.rs

@@ -2898,7 +2898,7 @@ impl<'tcx> TyCtxt<'tcx> {
             (ImplPolarity::Negative, ImplPolarity::Negative) => {}
         };
 
-        let is_marker_overlap = if self.features().overlapping_marker_traits {
+        let is_marker_overlap = if self.features().on(sym::overlapping_marker_traits) {
             let trait1_is_empty = self.impl_trait_ref(def_id1)
                 .map_or(false, |trait_ref| {
                     self.associated_item_def_ids(trait_ref.def_id).is_empty()

src/librustc/ty/wf.rs

@@ -5,7 +5,7 @@ use crate::ty::subst::SubstsRef;
 use crate::traits::{self, AssocTypeBoundData};
 use crate::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable};
 use std::iter::once;
-use syntax::symbol::{kw, Ident};
+use syntax::symbol::{kw, sym, Ident};
 use syntax_pos::Span;
 use crate::middle::lang_items;
 
@@ -538,7 +538,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
                     // checking those
 
                     let defer_to_coercion =
-                        self.infcx.tcx.features().object_safe_for_dispatch;
+                        self.infcx.tcx.features().on(sym::object_safe_for_dispatch);
 
                     if !defer_to_coercion {
                         let cause = self.cause(traits::MiscObligation);

src/librustc_codegen_utils/symbol_names_test.rs

@@ -15,7 +15,7 @@ pub fn report_symbol_names(tcx: TyCtxt<'_>) {
     // if the `rustc_attrs` feature is not enabled, then the
     // attributes we are interested in cannot be present anyway, so
     // skip the walk.
-    if !tcx.features().rustc_attrs {
+    if !tcx.features().on(sym::rustc_attrs) {
         return;
     }
 

src/librustc_feature/active.rs

@@ -6,34 +6,22 @@ use syntax_pos::edition::Edition;
 use syntax_pos::Span;
 use syntax_pos::symbol::{Symbol, sym};
 
-macro_rules! set {
-    ($field: ident) => {{
-        fn f(features: &mut Features, _: Span) {
-            features.$field = true;
-        }
-        f as fn(&mut Features, Span)
-    }}
-}
-
 macro_rules! declare_features {
     ($(
         $(#[doc = $doc:tt])* (active, $feature:ident, $ver:expr, $issue:expr, $edition:expr),
     )+) => {
         /// Represents active features that are currently being implemented or
         /// currently being considered for addition/removal.
-        pub const ACTIVE_FEATURES:
-            &[Feature] =
-            &[$(
-                // (sym::$feature, $ver, $issue, $edition, set!($feature))
-                Feature {
-                    state: State::Active { set: set!($feature) },
-                    name: sym::$feature,
-                    since: $ver,
-                    issue: $issue,
-                    edition: $edition,
-                    description: concat!($($doc,)*),
-                }
-            ),+];
+        pub const ACTIVE_FEATURES: &[Feature] = &[$(
+            Feature {
+                state: State::Active,
+                name: sym::$feature,
+                since: $ver,
+                issue: $issue,
+                edition: $edition,
+                description: concat!($($doc,)*),
+            }
+        ),+];
 
         /// A set of features to be used by later passes.
         #[derive(Clone, Default)]
@@ -44,28 +32,43 @@ macro_rules! declare_features {
             pub declared_lib_features: Vec<(Symbol, Span)>,
             $(
                 $(#[doc = $doc])*
-                pub $feature: bool
+                $feature: bool
             ),+
         }
 
         impl Features {
+            /// Is the given feature enabled?
+            ///
+            /// Panics if the symbol doesn't correspond to a declared feature.
+            pub fn on(&self, name: Symbol) -> bool {
+                match name {
+                    $(sym::$feature => self.$feature,)+
+                    _ => panic!("unknown feature `{}`", name),
+                }
+            }
+
+            /// Enable the given `feature`.
+            ///
+            /// Panics if called on a non-active feature
+            /// or a symbol not corresponding to a declared feature.
+            pub fn enable(&mut self, feature: &Feature, _: Span) {
+                let Feature { name, .. } = feature;
+                match feature.state {
+                    State::Active => match *name {
+                        $(sym::$feature => self.$feature = true,)+
+                        _ => panic!("unknown feature `{}`", name),
+                    },
+                    _ => panic!("called `set` on feature `{}` which is not `active`", name),
+                }
+            }
+
             pub fn walk_feature_fields(&self, mut f: impl FnMut(&str, bool)) {
                 $(f(stringify!($feature), self.$feature);)+
             }
         }
     };
 }
 
-impl Feature {
-    /// Sets this feature in `Features`. Panics if called on a non-active feature.
-    pub fn set(&self, features: &mut Features, span: Span) {
-        match self.state {
-            State::Active { set } => set(features, span),
-            _ => panic!("called `set` on feature `{}` which is not `active`", self.name)
-        }
-    }
-}
-
 // If you change this, please modify `src/doc/unstable-book` as well.
 //
 // Don't ever remove anything from this list; move them to `removed.rs`.