Emit check-cfg lints during attribute parsing rather than evaluation#149215

Author: JonathanBrouwer

compiler/rustc_attr_parsing/src/attributes/cfg.rs

@@ -2,16 +2,16 @@ use std::convert::identity;
 
 use rustc_ast::token::Delimiter;
 use rustc_ast::tokenstream::DelimSpan;
-use rustc_ast::{AttrItem, Attribute, CRATE_NODE_ID, LitKind, NodeId, ast, token};
+use rustc_ast::{AttrItem, Attribute, CRATE_NODE_ID, LitKind, ast, token};
 use rustc_errors::{Applicability, PResult};
 use rustc_feature::{AttrSuggestionStyle, AttributeTemplate, Features, template};
 use rustc_hir::attrs::CfgEntry;
+use rustc_hir::lints::AttributeLintKind;
 use rustc_hir::{AttrPath, RustcVersion};
 use rustc_parse::parser::{ForceCollect, Parser};
 use rustc_parse::{exp, parse_in};
 use rustc_session::Session;
 use rustc_session::config::ExpectedValues;
-use rustc_session::lint::BuiltinLintDiag;
 use rustc_session::lint::builtin::UNEXPECTED_CFGS;
 use rustc_session::parse::{ParseSess, feature_err};
 use rustc_span::{ErrorGuaranteed, Span, Symbol, sym};
@@ -23,10 +23,7 @@ use crate::session_diagnostics::{
     AttributeParseError, AttributeParseErrorReason, CfgAttrBadDelim, MetaBadDelimSugg,
     ParsedDescription,
 };
-use crate::{
-    AttributeParser, CfgMatchesLintEmitter, fluent_generated, parse_version, session_diagnostics,
-    try_gate_cfg,
-};
+use crate::{AttributeParser, fluent_generated, parse_version, session_diagnostics, try_gate_cfg};
 
 pub const CFG_TEMPLATE: AttributeTemplate = template!(
     List: &["predicate"],
@@ -195,43 +192,46 @@ fn parse_name_value<S: Stage>(
         }
     };
 
-    Ok(CfgEntry::NameValue { name, name_span, value, span })
+    match cx.sess.psess.check_config.expecteds.get(&name) {
+        Some(ExpectedValues::Some(values)) if !values.contains(&value.map(|(v, _)| v)) => cx
+            .emit_lint(
+                UNEXPECTED_CFGS,
+                AttributeLintKind::UnexpectedCfgValue((name, name_span), value),
+                span,
+            ),
+        None if cx.sess.psess.check_config.exhaustive_names => cx.emit_lint(
+            UNEXPECTED_CFGS,
+            AttributeLintKind::UnexpectedCfgName((name, name_span), value),
+            span,
+        ),
+        _ => { /* not unexpected */ }
+    }
+
+    Ok(CfgEntry::NameValue { name, value: value.map(|(v, _)| v), span })
 }
 
-pub fn eval_config_entry(
-    sess: &Session,
-    cfg_entry: &CfgEntry,
-    id: NodeId,
-    emit_lints: ShouldEmit,
-) -> EvalConfigResult {
+pub fn eval_config_entry(sess: &Session, cfg_entry: &CfgEntry) -> EvalConfigResult {
     match cfg_entry {
         CfgEntry::All(subs, ..) => {
-            let mut all = None;
             for sub in subs {
-                let res = eval_config_entry(sess, sub, id, emit_lints);
-                // We cannot short-circuit because `eval_config_entry` emits some lints
+                let res = eval_config_entry(sess, sub);
                 if !res.as_bool() {
-                    all.get_or_insert(res);
+                    return res;
                 }
             }
-            all.unwrap_or_else(|| EvalConfigResult::True)
+            EvalConfigResult::True
         }
         CfgEntry::Any(subs, span) => {
-            let mut any = None;
             for sub in subs {
-                let res = eval_config_entry(sess, sub, id, emit_lints);
-                // We cannot short-circuit because `eval_config_entry` emits some lints
+                let res = eval_config_entry(sess, sub);
                 if res.as_bool() {
-                    any.get_or_insert(res);
+                    return res;
                 }
             }
-            any.unwrap_or_else(|| EvalConfigResult::False {
-                reason: cfg_entry.clone(),
-                reason_span: *span,
-            })
+            EvalConfigResult::False { reason: cfg_entry.clone(), reason_span: *span }
         }
         CfgEntry::Not(sub, span) => {
-            if eval_config_entry(sess, sub, id, emit_lints).as_bool() {
+            if eval_config_entry(sess, sub).as_bool() {
                 EvalConfigResult::False { reason: cfg_entry.clone(), reason_span: *span }
             } else {
                 EvalConfigResult::True
@@ -244,32 +244,8 @@ pub fn eval_config_entry(
                 EvalConfigResult::False { reason: cfg_entry.clone(), reason_span: *span }
             }
         }
-        CfgEntry::NameValue { name, name_span, value, span } => {
-            if let ShouldEmit::ErrorsAndLints = emit_lints {
-                match sess.psess.check_config.expecteds.get(name) {
-                    Some(ExpectedValues::Some(values))
-                        if !values.contains(&value.map(|(v, _)| v)) =>
-                    {
-                        id.emit_span_lint(
-                            sess,
-                            UNEXPECTED_CFGS,
-                            *span,
-                            BuiltinLintDiag::UnexpectedCfgValue((*name, *name_span), *value),
-                        );
-                    }
-                    None if sess.psess.check_config.exhaustive_names => {
-                        id.emit_span_lint(
-                            sess,
-                            UNEXPECTED_CFGS,
-                            *span,
-                            BuiltinLintDiag::UnexpectedCfgName((*name, *name_span), *value),
-                        );
-                    }
-                    _ => { /* not unexpected */ }
-                }
-            }
-
-            if sess.psess.config.contains(&(*name, value.map(|(v, _)| v))) {
+        CfgEntry::NameValue { name, value, span } => {
+            if sess.psess.config.contains(&(*name, *value)) {
                 EvalConfigResult::True
             } else {
                 EvalConfigResult::False { reason: cfg_entry.clone(), reason_span: *span }

compiler/rustc_attr_parsing/src/attributes/cfg_old.rs

@@ -2,6 +2,7 @@ use rustc_ast::{LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit, Nod
 use rustc_ast_pretty::pprust;
 use rustc_feature::{Features, GatedCfg, find_gated_cfg};
 use rustc_hir::RustcVersion;
+use rustc_hir::lints::AttributeLintKind;
 use rustc_session::Session;
 use rustc_session::config::ExpectedValues;
 use rustc_session::lint::builtin::UNEXPECTED_CFGS;
@@ -51,21 +52,21 @@ pub fn cfg_matches(
                     sess,
                     UNEXPECTED_CFGS,
                     cfg.span,
-                    BuiltinLintDiag::UnexpectedCfgValue(
+                    BuiltinLintDiag::AttributeLint(AttributeLintKind::UnexpectedCfgValue(
                         (cfg.name, cfg.name_span),
                         cfg.value.map(|v| (v, cfg.value_span.unwrap())),
-                    ),
+                    )),
                 );
             }
             None if sess.psess.check_config.exhaustive_names => {
                 lint_emitter.emit_span_lint(
                     sess,
                     UNEXPECTED_CFGS,
                     cfg.span,
-                    BuiltinLintDiag::UnexpectedCfgName(
+                    BuiltinLintDiag::AttributeLint(AttributeLintKind::UnexpectedCfgName(
                         (cfg.name, cfg.name_span),
                         cfg.value.map(|v| (v, cfg.value_span.unwrap())),
-                    ),
+                    )),
                 );
             }
             _ => { /* not unexpected */ }

compiler/rustc_builtin_macros/src/cfg.rs

@@ -26,13 +26,7 @@ pub(crate) fn expand_cfg(
 
     ExpandResult::Ready(match parse_cfg(cx, sp, tts) {
         Ok(cfg) => {
-            let matches_cfg = attr::eval_config_entry(
-                cx.sess,
-                &cfg,
-                cx.current_expansion.lint_node_id,
-                ShouldEmit::ErrorsAndLints,
-            )
-            .as_bool();
+            let matches_cfg = attr::eval_config_entry(cx.sess, &cfg).as_bool();
 
             MacEager::expr(cx.expr_bool(sp, matches_cfg))
         }

compiler/rustc_builtin_macros/src/cfg_select.rs

@@ -1,7 +1,7 @@
 use rustc_ast::tokenstream::TokenStream;
 use rustc_attr_parsing as attr;
 use rustc_attr_parsing::{
-    CfgSelectBranches, CfgSelectPredicate, EvalConfigResult, ShouldEmit, parse_cfg_select,
+    CfgSelectBranches, CfgSelectPredicate, EvalConfigResult, parse_cfg_select,
 };
 use rustc_expand::base::{DummyResult, ExpandResult, ExtCtxt, MacroExpanderResult};
 use rustc_span::{Ident, Span, sym};
@@ -10,21 +10,13 @@ use crate::errors::{CfgSelectNoMatches, CfgSelectUnreachable};
 
 /// Selects the first arm whose predicate evaluates to true.
 fn select_arm(ecx: &ExtCtxt<'_>, branches: CfgSelectBranches) -> Option<(TokenStream, Span)> {
-    let mut result = None;
     for (cfg, tt, arm_span) in branches.reachable {
-        if let EvalConfigResult::True = attr::eval_config_entry(
-            &ecx.sess,
-            &cfg,
-            ecx.current_expansion.lint_node_id,
-            ShouldEmit::ErrorsAndLints,
-        ) {
-            // FIXME(#149215) Ideally we should short-circuit here, but `eval_config_entry` currently emits lints so we cannot do this yet.
-            result.get_or_insert((tt, arm_span));
+        if let EvalConfigResult::True = attr::eval_config_entry(&ecx.sess, &cfg) {
+            return Some((tt, arm_span));
         }
     }
 
-    let wildcard = branches.wildcard.map(|(_, tt, span)| (tt, span));
-    result.or(wildcard)
+    branches.wildcard.map(|(_, tt, span)| (tt, span))
 }
 
 pub(super) fn expand_cfg_select<'cx>(

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -13,8 +13,7 @@ use find_msvc_tools;
 use itertools::Itertools;
 use regex::Regex;
 use rustc_arena::TypedArena;
-use rustc_ast::CRATE_NODE_ID;
-use rustc_attr_parsing::{ShouldEmit, eval_config_entry};
+use rustc_attr_parsing::eval_config_entry;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_data_structures::memmap::Mmap;
 use rustc_data_structures::temp_dir::MaybeTempDir;
@@ -3029,9 +3028,7 @@ fn add_dynamic_crate(cmd: &mut dyn Linker, sess: &Session, cratepath: &Path) {
 
 fn relevant_lib(sess: &Session, lib: &NativeLib) -> bool {
     match lib.cfg {
-        Some(ref cfg) => {
-            eval_config_entry(sess, cfg, CRATE_NODE_ID, ShouldEmit::ErrorsAndLints).as_bool()
-        }
+        Some(ref cfg) => eval_config_entry(sess, cfg).as_bool(),
         None => true,
     }
 }

compiler/rustc_expand/src/config.rs

@@ -163,10 +163,7 @@ pub fn pre_configure_attrs(sess: &Session, attrs: &[Attribute]) -> ast::AttrVec
         .iter()
         .flat_map(|attr| strip_unconfigured.process_cfg_attr(attr))
         .take_while(|attr| {
-            !is_cfg(attr)
-                || strip_unconfigured
-                    .cfg_true(attr, strip_unconfigured.lint_node_id, ShouldEmit::Nothing)
-                    .as_bool()
+            !is_cfg(attr) || strip_unconfigured.cfg_true(attr, ShouldEmit::Nothing).as_bool()
         })
         .collect()
 }
@@ -309,14 +306,7 @@ impl<'a> StripUnconfigured<'a> {
             );
         }
 
-        if !attr::eval_config_entry(
-            self.sess,
-            &cfg_predicate,
-            ast::CRATE_NODE_ID,
-            ShouldEmit::ErrorsAndLints,
-        )
-        .as_bool()
-        {
+        if !attr::eval_config_entry(self.sess, &cfg_predicate).as_bool() {
             return vec![trace_attr];
         }
 
@@ -400,23 +390,17 @@ impl<'a> StripUnconfigured<'a> {
 
     /// Determines if a node with the given attributes should be included in this configuration.
     fn in_cfg(&self, attrs: &[Attribute]) -> bool {
-        attrs.iter().all(|attr| {
-            !is_cfg(attr)
-                || self.cfg_true(attr, self.lint_node_id, ShouldEmit::ErrorsAndLints).as_bool()
-        })
+        attrs
+            .iter()
+            .all(|attr| !is_cfg(attr) || self.cfg_true(attr, ShouldEmit::ErrorsAndLints).as_bool())
     }
 
-    pub(crate) fn cfg_true(
-        &self,
-        attr: &Attribute,
-        node: NodeId,
-        emit_errors: ShouldEmit,
-    ) -> EvalConfigResult {
+    pub(crate) fn cfg_true(&self, attr: &Attribute, emit_errors: ShouldEmit) -> EvalConfigResult {
         let Some(cfg) = AttributeParser::parse_single(
             self.sess,
             attr,
             attr.span,
-            node,
+            self.lint_node_id,
             self.features,
             emit_errors,
             parse_cfg,
@@ -426,7 +410,7 @@ impl<'a> StripUnconfigured<'a> {
             return EvalConfigResult::True;
         };
 
-        eval_config_entry(self.sess, &cfg, self.lint_node_id, emit_errors)
+        eval_config_entry(self.sess, &cfg)
     }
 
     /// If attributes are not allowed on expressions, emit an error for `attr`

compiler/rustc_expand/src/expand.rs

@@ -2213,7 +2213,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
         attr: ast::Attribute,
         pos: usize,
     ) -> EvalConfigResult {
-        let res = self.cfg().cfg_true(&attr, node.node_id(), ShouldEmit::ErrorsAndLints);
+        let res = self.cfg().cfg_true(&attr, ShouldEmit::ErrorsAndLints);
         if res.as_bool() {
             // A trace attribute left in AST in place of the original `cfg` attribute.
             // It can later be used by lints or other diagnostics.

compiler/rustc_hir/src/attrs/data_structures.rs

@@ -190,7 +190,7 @@ pub enum CfgEntry {
     Any(ThinVec<CfgEntry>, Span),
     Not(Box<CfgEntry>, Span),
     Bool(bool, Span),
-    NameValue { name: Symbol, name_span: Span, value: Option<(Symbol, Span)>, span: Span },
+    NameValue { name: Symbol, value: Option<Symbol>, span: Span },
     Version(Option<RustcVersion>, Span),
 }
 

compiler/rustc_lint/src/early/diagnostics.rs

@@ -169,12 +169,6 @@ pub fn decorate_builtin_lint(
             }
             .decorate_lint(diag);
         }
-        BuiltinLintDiag::UnexpectedCfgName(name, value) => {
-            check_cfg::unexpected_cfg_name(sess, tcx, name, value).decorate_lint(diag);
-        }
-        BuiltinLintDiag::UnexpectedCfgValue(name, value) => {
-            check_cfg::unexpected_cfg_value(sess, tcx, name, value).decorate_lint(diag);
-        }
         BuiltinLintDiag::DeprecatedWhereclauseLocation(left_sp, sugg) => {
             let suggestion = match sugg {
                 Some((right_sp, sugg)) => lints::DeprecatedWhereClauseLocationSugg::MoveToEnd {
@@ -307,8 +301,8 @@ pub fn decorate_builtin_lint(
 }
 
 pub fn decorate_attribute_lint(
-    _sess: &Session,
-    _tcx: Option<TyCtxt<'_>>,
+    sess: &Session,
+    tcx: Option<TyCtxt<'_>>,
     kind: &AttributeLintKind,
     diag: &mut Diag<'_, ()>,
 ) {
@@ -364,5 +358,11 @@ pub fn decorate_attribute_lint(
             suggestion: lints::UnsafeAttrOutsideUnsafeSuggestion { left, right },
         }
         .decorate_lint(diag),
+        &AttributeLintKind::UnexpectedCfgName(name, value) => {
+            check_cfg::unexpected_cfg_name(sess, tcx, name, value).decorate_lint(diag)
+        }
+        &AttributeLintKind::UnexpectedCfgValue(name, value) => {
+            check_cfg::unexpected_cfg_value(sess, tcx, name, value).decorate_lint(diag)
+        }
     }
 }

compiler/rustc_lint_defs/src/lib.rs

@@ -636,8 +636,6 @@ pub enum BuiltinLintDiag {
     },
     BreakWithLabelAndLoop(Span),
     UnicodeTextFlow(Span, String),
-    UnexpectedCfgName((Symbol, Span), Option<(Symbol, Span)>),
-    UnexpectedCfgValue((Symbol, Span), Option<(Symbol, Span)>),
     DeprecatedWhereclauseLocation(Span, Option<(Span, String)>),
     SingleUseLifetime {
         /// Span of the parameter which declares this lifetime.
@@ -732,6 +730,8 @@ pub enum AttributeLintKind {
         attribute_name_span: Span,
         sugg_spans: (Span, Span),
     },
+    UnexpectedCfgName((Symbol, Span), Option<(Symbol, Span)>),
+    UnexpectedCfgValue((Symbol, Span), Option<(Symbol, Span)>),
 }
 
 pub type RegisteredTools = FxIndexSet<Ident>;