Skip to content
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

Improve allowness of the unexpected_cfgs lint #94433

Merged
merged 1 commit into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
//! Parsing and validation of builtin attributes

use rustc_ast as ast;
use rustc_ast::node_id::CRATE_NODE_ID;
use rustc_ast::{Attribute, Lit, LitKind, MetaItem, MetaItemKind, NestedMetaItem};
use rustc_ast::{Attribute, Lit, LitKind, MetaItem, MetaItemKind, NestedMetaItem, NodeId};
use rustc_ast_pretty::pprust;
use rustc_errors::{struct_span_err, Applicability};
use rustc_feature::{find_gated_cfg, is_builtin_attr_name, Features, GatedCfg};
Expand Down Expand Up @@ -436,7 +435,12 @@ pub fn find_crate_name(sess: &Session, attrs: &[Attribute]) -> Option<Symbol> {
}

/// Tests if a cfg-pattern matches the cfg set
pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Features>) -> bool {
pub fn cfg_matches(
cfg: &ast::MetaItem,
sess: &ParseSess,
lint_node_id: NodeId,
features: Option<&Features>,
) -> bool {
eval_condition(cfg, sess, features, &mut |cfg| {
try_gate_cfg(cfg, sess, features);
let error = |span, msg| {
Expand Down Expand Up @@ -470,7 +474,7 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
sess.buffer_lint_with_diagnostic(
UNEXPECTED_CFGS,
cfg.span,
CRATE_NODE_ID,
lint_node_id,
"unexpected `cfg` condition name",
BuiltinLintDiagnostics::UnexpectedCfg(ident.span, name, None),
);
Expand All @@ -482,7 +486,7 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
sess.buffer_lint_with_diagnostic(
UNEXPECTED_CFGS,
cfg.span,
CRATE_NODE_ID,
lint_node_id,
"unexpected `cfg` condition value",
BuiltinLintDiagnostics::UnexpectedCfg(
cfg.name_value_literal_span().unwrap(),
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_builtin_macros/src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ pub fn expand_cfg(

match parse_cfg(cx, sp, tts) {
Ok(cfg) => {
let matches_cfg = attr::cfg_matches(&cfg, &cx.sess.parse_sess, cx.ecfg.features);
let matches_cfg = attr::cfg_matches(
&cfg,
&cx.sess.parse_sess,
cx.current_expansion.lint_node_id,
cx.ecfg.features,
);
MacEager::expr(cx.expr_bool(sp, matches_cfg))
}
Err(mut err) => {
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_builtin_macros/src/cfg_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_ast::mut_visit::MutVisitor;
use rustc_ast::ptr::P;
use rustc_ast::tokenstream::CanSynthesizeMissingTokens;
use rustc_ast::visit::Visitor;
use rustc_ast::NodeId;
use rustc_ast::{mut_visit, visit};
use rustc_ast::{AstLike, Attribute};
use rustc_expand::base::{Annotatable, ExtCtxt};
Expand All @@ -26,15 +27,16 @@ crate fn expand(
) -> Vec<Annotatable> {
check_builtin_macro_attribute(ecx, meta_item, sym::cfg_eval);
warn_on_duplicate_attribute(&ecx, &annotatable, sym::cfg_eval);
vec![cfg_eval(ecx.sess, ecx.ecfg.features, annotatable)]
vec![cfg_eval(ecx.sess, ecx.ecfg.features, annotatable, ecx.current_expansion.lint_node_id)]
}

crate fn cfg_eval(
sess: &Session,
features: Option<&Features>,
annotatable: Annotatable,
lint_node_id: NodeId,
) -> Annotatable {
CfgEval { cfg: &mut StripUnconfigured { sess, features, config_tokens: true } }
CfgEval { cfg: &mut StripUnconfigured { sess, features, config_tokens: true, lint_node_id } }
.configure_annotatable(annotatable)
// Since the item itself has already been configured by the `InvocationCollector`,
// we know that fold result vector will contain exactly one element.
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_builtin_macros/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,12 @@ impl MultiItemModifier for Expander {
match &mut resolutions[..] {
[] => {}
[(_, first_item, _), others @ ..] => {
*first_item = cfg_eval(sess, features, item.clone());
*first_item = cfg_eval(
sess,
features,
item.clone(),
ecx.current_expansion.lint_node_id,
);
for (_, item, _) in others {
*item = first_item.clone();
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_arena::TypedArena;
use rustc_ast::CRATE_NODE_ID;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::memmap::Mmap;
use rustc_data_structures::temp_dir::MaybeTempDir;
Expand Down Expand Up @@ -2434,7 +2435,7 @@ fn add_upstream_native_libraries(

fn relevant_lib(sess: &Session, lib: &NativeLib) -> bool {
match lib.cfg {
Some(ref cfg) => rustc_attr::cfg_matches(cfg, &sess.parse_sess, None),
Some(ref cfg) => rustc_attr::cfg_matches(cfg, &sess.parse_sess, CRATE_NODE_ID, None),
None => true,
}
}
Expand Down
20 changes: 16 additions & 4 deletions compiler/rustc_expand/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_ast::token::{DelimToken, Token, TokenKind};
use rustc_ast::tokenstream::{AttrAnnotatedTokenStream, AttrAnnotatedTokenTree};
use rustc_ast::tokenstream::{DelimSpan, Spacing};
use rustc_ast::tokenstream::{LazyTokenStream, TokenTree};
use rustc_ast::NodeId;
use rustc_ast::{self as ast, AstLike, AttrStyle, Attribute, MetaItem};
use rustc_attr as attr;
use rustc_data_structures::fx::FxHashMap;
Expand All @@ -29,6 +30,7 @@ pub struct StripUnconfigured<'a> {
/// This is only used for the input to derive macros,
/// which needs eager expansion of `cfg` and `cfg_attr`
pub config_tokens: bool,
pub lint_node_id: NodeId,
}

fn get_features(
Expand Down Expand Up @@ -196,8 +198,13 @@ fn get_features(
}

// `cfg_attr`-process the crate's attributes and compute the crate's features.
pub fn features(sess: &Session, mut krate: ast::Crate) -> (ast::Crate, Features) {
let mut strip_unconfigured = StripUnconfigured { sess, features: None, config_tokens: false };
pub fn features(
sess: &Session,
mut krate: ast::Crate,
lint_node_id: NodeId,
) -> (ast::Crate, Features) {
let mut strip_unconfigured =
StripUnconfigured { sess, features: None, config_tokens: false, lint_node_id };

let unconfigured_attrs = krate.attrs.clone();
let diag = &sess.parse_sess.span_diagnostic;
Expand Down Expand Up @@ -353,7 +360,12 @@ impl<'a> StripUnconfigured<'a> {
);
}

if !attr::cfg_matches(&cfg_predicate, &self.sess.parse_sess, self.features) {
if !attr::cfg_matches(
&cfg_predicate,
&self.sess.parse_sess,
self.lint_node_id,
self.features,
) {
return vec![];
}

Expand Down Expand Up @@ -445,7 +457,7 @@ impl<'a> StripUnconfigured<'a> {
}
};
parse_cfg(&meta_item, &self.sess).map_or(true, |meta_item| {
attr::cfg_matches(&meta_item, &self.sess.parse_sess, self.features)
attr::cfg_matches(&meta_item, &self.sess.parse_sess, self.lint_node_id, self.features)
})
}

Expand Down
23 changes: 13 additions & 10 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,11 +551,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
// attribute is expanded. Therefore, we don't need to configure the tokens
// Derive macros *can* see the results of cfg-expansion - they are handled
// specially in `fully_expand_fragment`
cfg: StripUnconfigured {
sess: &self.cx.sess,
features: self.cx.ecfg.features,
config_tokens: false,
},
cx: self.cx,
invocations: Vec::new(),
monotonic: self.monotonic,
Expand Down Expand Up @@ -1537,12 +1532,20 @@ impl InvocationCollectorNode for AstLikeWrapper<P<ast::Expr>, OptExprTag> {

struct InvocationCollector<'a, 'b> {
cx: &'a mut ExtCtxt<'b>,
cfg: StripUnconfigured<'a>,
invocations: Vec<(Invocation, Option<Lrc<SyntaxExtension>>)>,
monotonic: bool,
}

impl<'a, 'b> InvocationCollector<'a, 'b> {
fn cfg(&self) -> StripUnconfigured<'_> {
StripUnconfigured {
sess: &self.cx.sess,
features: self.cx.ecfg.features,
config_tokens: false,
lint_node_id: self.cx.current_expansion.lint_node_id,
}
}

fn collect(&mut self, fragment_kind: AstFragmentKind, kind: InvocationKind) -> AstFragment {
let expn_id = LocalExpnId::fresh_empty();
let vis = kind.placeholder_visibility();
Expand Down Expand Up @@ -1682,7 +1685,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
attr: ast::Attribute,
pos: usize,
) -> bool {
let res = self.cfg.cfg_true(&attr);
let res = self.cfg().cfg_true(&attr);
if res {
// FIXME: `cfg(TRUE)` attributes do not currently remove themselves during expansion,
// and some tools like rustdoc and clippy rely on that. Find a way to remove them
Expand All @@ -1695,7 +1698,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {

fn expand_cfg_attr(&self, node: &mut impl AstLike, attr: ast::Attribute, pos: usize) {
node.visit_attrs(|attrs| {
attrs.splice(pos..pos, self.cfg.expand_cfg_attr(attr, false));
attrs.splice(pos..pos, self.cfg().expand_cfg_attr(attr, false));
});
}

Expand All @@ -1717,7 +1720,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
continue;
}
_ => {
Node::pre_flat_map_node_collect_attr(&self.cfg, &attr);
Node::pre_flat_map_node_collect_attr(&self.cfg(), &attr);
self.collect_attr((attr, pos, derives), node.to_annotatable(), Node::KIND)
.make_ast::<Node>()
}
Expand Down Expand Up @@ -1881,7 +1884,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
fn visit_expr(&mut self, node: &mut P<ast::Expr>) {
// FIXME: Feature gating is performed inconsistently between `Expr` and `OptExpr`.
if let Some(attr) = node.attrs.first() {
self.cfg.maybe_emit_expr_attr_err(attr);
self.cfg().maybe_emit_expr_attr_err(attr);
}
self.visit_node(node)
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::interface::{Compiler, Result};
use crate::proc_macro_decls;
use crate::util;

use ast::CRATE_NODE_ID;
use rustc_ast::mut_visit::MutVisitor;
use rustc_ast::{self as ast, visit};
use rustc_borrowck as mir_borrowck;
Expand Down Expand Up @@ -188,7 +189,7 @@ pub fn register_plugins<'a>(
)
});

let (krate, features) = rustc_expand::config::features(sess, krate);
let (krate, features) = rustc_expand::config::features(sess, krate, CRATE_NODE_ID);
// these need to be set "early" so that expansion sees `quote` if enabled.
sess.init_features(features);

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_metadata/src/native_libs.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rustc_ast::CRATE_NODE_ID;
use rustc_attr as attr;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::struct_span_err;
Expand All @@ -21,7 +22,7 @@ crate fn collect(tcx: TyCtxt<'_>) -> Vec<NativeLib> {

crate fn relevant_lib(sess: &Session, lib: &NativeLib) -> bool {
match lib.cfg {
Some(ref cfg) => attr::cfg_matches(cfg, &sess.parse_sess, None),
Some(ref cfg) => attr::cfg_matches(cfg, &sess.parse_sess, CRATE_NODE_ID, None),
None => true,
}
}
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/check-cfg/allow-macro-cfg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// This test check that local #[allow(unexpected_cfgs)] works
//
// check-pass
// compile-flags:--check-cfg=names() -Z unstable-options

#[allow(unexpected_cfgs)]
fn foo() {
if cfg!(FALSE) {}
}

fn main() {
#[allow(unexpected_cfgs)]
if cfg!(FALSE) {}
}
11 changes: 11 additions & 0 deletions src/test/ui/check-cfg/allow-same-level.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// This test check that #[allow(unexpected_cfgs)] doesn't work if put on the same level
//
// check-pass
// compile-flags:--check-cfg=names() -Z unstable-options

#[allow(unexpected_cfgs)]
#[cfg(FALSE)]
//~^ WARNING unexpected `cfg` condition name
fn bar() {}

fn main() {}
10 changes: 10 additions & 0 deletions src/test/ui/check-cfg/allow-same-level.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: unexpected `cfg` condition name
--> $DIR/allow-same-level.rs:7:7
|
LL | #[cfg(FALSE)]
| ^^^^^
|
= note: `#[warn(unexpected_cfgs)]` on by default

warning: 1 warning emitted

15 changes: 15 additions & 0 deletions src/test/ui/check-cfg/allow-top-level.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// This test check that a top-level #![allow(unexpected_cfgs)] works
//
// check-pass
// compile-flags:--check-cfg=names() -Z unstable-options

#![allow(unexpected_cfgs)]

#[cfg(FALSE)]
fn bar() {}

fn foo() {
if cfg!(FALSE) {}
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/check-cfg/allow-upper-level.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// This test check that #[allow(unexpected_cfgs)] work if put on an upper level
//
// check-pass
// compile-flags:--check-cfg=names() -Z unstable-options

#[allow(unexpected_cfgs)]
mod aa {
#[cfg(FALSE)]
fn bar() {}
}

fn main() {}