Skip to content

Commit

Permalink
Rollup merge of rust-lang#94433 - Urgau:check-cfg-allowness, r=petroc…
Browse files Browse the repository at this point in the history
…henkov

Improve allowness of the unexpected_cfgs lint

This pull-request improve the allowness (`#[allow(...)]`) of the `unexpected_cfgs` lint.

Before this PR only crate level `#![allow(unexpected_cfgs)]` worked, now with this PR it also work when put around `cfg!` or if it is in a upper level. Making it work ~for the attributes `cfg`, `cfg_attr`, ...~ for the same level is awkward as the current code is design to give "Some parent node that is close to this macro call" (cf. https://doc.rust-lang.org/nightly/nightly-rustc/rustc_expand/base/struct.ExpansionData.html) meaning that allow on the same line as an attribute won't work. I'm note even sure if this would be possible.

Found while working on rust-lang#94298.
r? ```````@petrochenkov```````
  • Loading branch information
Dylan-DPC committed Mar 2, 2022
2 parents c168107 + 765205b commit 8df5cb9
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 26 deletions.
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 @@ -1538,12 +1533,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 @@ -1683,7 +1686,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 @@ -1696,7 +1699,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 @@ -1718,7 +1721,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 @@ -1882,7 +1885,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() {}

0 comments on commit 8df5cb9

Please sign in to comment.