Skip to content

Simplify gated cfg checking #34272

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 17, 2016
Merged

Simplify gated cfg checking #34272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 12 additions & 36 deletions src/librustc_driver/driver.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,15 +577,13 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
//
// baz! should not use this definition unless foo is enabled.

let mut feature_gated_cfgs = vec![];
krate = time(time_passes, "configuration 1", || {
sess.track_errors(|| {
syntax::config::strip_unconfigured_items(sess.diagnostic(),
krate,
sess.opts.test,
&mut feature_gated_cfgs)
})
})?;
krate = time(time_passes, "configuration", || {
let (krate, features) =
syntax::config::strip_unconfigured_items(krate, &sess.parse_sess, sess.opts.test);
// these need to be set "early" so that expansion sees `quote` if enabled.
*sess.features.borrow_mut() = features;
krate
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to stop compilation if there were errors here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason to stop compilation -- the only errors that get reported here are malformed cfg attributes (in which case the item is kept), malformed cfg_attr attributes (in which case the attribute is removed), and malformed feature attributes (in which case we might not see the features).
At worst, the user might see some gated feature errors for a feature they intended to use.

Copy link
Contributor Author

@jseyfried jseyfried Jun 16, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also, gated cfg errors are reported, which definitely don't require us to stop compilation)


*sess.crate_types.borrow_mut() = collect_crate_types(sess, &krate.attrs);
sess.crate_disambiguator.set(token::intern(&compute_crate_disambiguator(sess)));
Expand All @@ -594,13 +592,6 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
middle::recursion_limit::update_recursion_limit(sess, &krate);
});

// these need to be set "early" so that expansion sees `quote` if enabled.
sess.track_errors(|| {
*sess.features.borrow_mut() =
syntax::feature_gate::get_features(&sess.parse_sess.span_diagnostic,
&krate);
})?;

krate = time(time_passes, "crate injection", || {
let alt_std_name = sess.opts.alt_std_name.clone();
syntax::std_inject::maybe_inject_crates_ref(&sess.parse_sess, krate, alt_std_name)
Expand Down Expand Up @@ -699,7 +690,6 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
let mut ecx = syntax::ext::base::ExtCtxt::new(&sess.parse_sess,
krate.config.clone(),
cfg,
&mut feature_gated_cfgs,
&mut loader);
syntax_ext::register_builtins(&mut ecx.syntax_env);
let (ret, macro_names) = syntax::ext::expand::expand_crate(ecx,
Expand All @@ -712,19 +702,6 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
ret
});

krate = sess.track_errors(|| {
time(time_passes, "gated configuration checking", || {
let features = sess.features.borrow();
feature_gated_cfgs.sort();
feature_gated_cfgs.dedup();
for cfg in &feature_gated_cfgs {
cfg.check_and_emit(sess.diagnostic(), &features, sess.codemap());
}
});

krate
})?;

krate = time(time_passes, "maybe building test harness", || {
syntax::test::modify_for_testing(&sess.parse_sess,
sess.opts.test,
Expand All @@ -739,12 +716,11 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
// Needs to go *after* expansion to be able to check the results of macro expansion.
time(time_passes, "complete gated feature checking", || {
sess.track_errors(|| {
let features = syntax::feature_gate::check_crate(sess.codemap(),
&sess.parse_sess.span_diagnostic,
&krate,
&attributes,
sess.opts.unstable_features);
*sess.features.borrow_mut() = features;
syntax::feature_gate::check_crate(&krate,
&sess.parse_sess,
&sess.features.borrow(),
&attributes,
sess.opts.unstable_features);
})
})?;

Expand Down
33 changes: 13 additions & 20 deletions src/libsyntax/attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ use ast::{Stmt, StmtKind, DeclKind};
use ast::{Expr, Item, Local, Decl};
use codemap::{Span, Spanned, spanned, dummy_spanned};
use codemap::BytePos;
use config::CfgDiag;
use errors::Handler;
use feature_gate::{GatedCfg, GatedCfgAttr};
use feature_gate::{Features, GatedCfg};
use parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration};
use parse::token::InternedString;
use parse::token;
use parse::{ParseSess, token};
use ptr::P;

use std::cell::{RefCell, Cell};
Expand Down Expand Up @@ -365,35 +364,29 @@ pub fn requests_inline(attrs: &[Attribute]) -> bool {
}

/// Tests if a cfg-pattern matches the cfg set
pub fn cfg_matches<T: CfgDiag>(cfgs: &[P<MetaItem>],
cfg: &ast::MetaItem,
diag: &mut T) -> bool {
pub fn cfg_matches(cfgs: &[P<MetaItem>], cfg: &ast::MetaItem,
sess: &ParseSess, features: Option<&Features>)
-> bool {
match cfg.node {
ast::MetaItemKind::List(ref pred, ref mis) if &pred[..] == "any" =>
mis.iter().any(|mi| cfg_matches(cfgs, &mi, diag)),
mis.iter().any(|mi| cfg_matches(cfgs, &mi, sess, features)),
ast::MetaItemKind::List(ref pred, ref mis) if &pred[..] == "all" =>
mis.iter().all(|mi| cfg_matches(cfgs, &mi, diag)),
mis.iter().all(|mi| cfg_matches(cfgs, &mi, sess, features)),
ast::MetaItemKind::List(ref pred, ref mis) if &pred[..] == "not" => {
if mis.len() != 1 {
diag.emit_error(|diagnostic| {
diagnostic.span_err(cfg.span, "expected 1 cfg-pattern");
});
sess.span_diagnostic.span_err(cfg.span, "expected 1 cfg-pattern");
return false;
}
!cfg_matches(cfgs, &mis[0], diag)
!cfg_matches(cfgs, &mis[0], sess, features)
}
ast::MetaItemKind::List(ref pred, _) => {
diag.emit_error(|diagnostic| {
diagnostic.span_err(cfg.span,
&format!("invalid predicate `{}`", pred));
});
sess.span_diagnostic.span_err(cfg.span, &format!("invalid predicate `{}`", pred));
false
},
ast::MetaItemKind::Word(_) | ast::MetaItemKind::NameValue(..) => {
diag.flag_gated(|feature_gated_cfgs| {
feature_gated_cfgs.extend(
GatedCfg::gate(cfg).map(GatedCfgAttr::GatedCfg));
});
if let (Some(features), Some(gated_cfg)) = (features, GatedCfg::gate(cfg)) {
gated_cfg.check_and_emit(sess, features);
}
contains(cfgs, cfg)
}
}
Expand Down
108 changes: 48 additions & 60 deletions src/libsyntax/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,24 @@
// except according to those terms.

use attr::{AttrMetaMethods, HasAttrs};
use errors::Handler;
use feature_gate::GatedCfgAttr;
use feature_gate::{emit_feature_err, EXPLAIN_STMT_ATTR_SYNTAX, Features, get_features, GateIssue};
use fold::Folder;
use {ast, fold, attr};
use codemap::{Spanned, respan};
use parse::token;
use parse::{ParseSess, token};
use ptr::P;

use util::small_vector::SmallVector;

/// A folder that strips out items that do not belong in the current configuration.
pub struct StripUnconfigured<'a> {
diag: CfgDiagReal<'a, 'a>,
should_test: bool,
config: &'a ast::CrateConfig,
pub config: &'a ast::CrateConfig,
pub should_test: bool,
pub sess: &'a ParseSess,
pub features: Option<&'a Features>,
}

impl<'a> StripUnconfigured<'a> {
pub fn new(config: &'a ast::CrateConfig,
should_test: bool,
diagnostic: &'a Handler,
feature_gated_cfgs: &'a mut Vec<GatedCfgAttr>)
-> Self {
StripUnconfigured {
config: config,
should_test: should_test,
diag: CfgDiagReal { diag: diagnostic, feature_gated_cfgs: feature_gated_cfgs },
}
}

fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
let node = self.process_cfg_attrs(node);
if self.in_cfg(node.attrs()) { Some(node) } else { None }
Expand All @@ -59,20 +47,20 @@ impl<'a> StripUnconfigured<'a> {
Some(attr_list) => attr_list,
None => {
let msg = "expected `#[cfg_attr(<cfg pattern>, <attr>)]`";
self.diag.diag.span_err(attr.span, msg);
self.sess.span_diagnostic.span_err(attr.span, msg);
return None;
}
};
let (cfg, mi) = match (attr_list.len(), attr_list.get(0), attr_list.get(1)) {
(2, Some(cfg), Some(mi)) => (cfg, mi),
_ => {
let msg = "expected `#[cfg_attr(<cfg pattern>, <attr>)]`";
self.diag.diag.span_err(attr.span, msg);
self.sess.span_diagnostic.span_err(attr.span, msg);
return None;
}
};

if attr::cfg_matches(self.config, &cfg, &mut self.diag) {
if attr::cfg_matches(self.config, &cfg, self.sess, self.features) {
self.process_cfg_attr(respan(mi.span, ast::Attribute_ {
id: attr::mk_attr_id(),
style: attr.node.style,
Expand All @@ -98,41 +86,55 @@ impl<'a> StripUnconfigured<'a> {
};

if mis.len() != 1 {
self.diag.emit_error(|diagnostic| {
diagnostic.span_err(attr.span, "expected 1 cfg-pattern");
});
self.sess.span_diagnostic.span_err(attr.span, "expected 1 cfg-pattern");
return true;
}

attr::cfg_matches(self.config, &mis[0], &mut self.diag)
attr::cfg_matches(self.config, &mis[0], self.sess, self.features)
})
}

// Visit attributes on expression and statements (but not attributes on items in blocks).
fn visit_stmt_or_expr_attrs(&mut self, attrs: &[ast::Attribute]) {
// flag the offending attributes
for attr in attrs.iter() {
self.diag.feature_gated_cfgs.push(GatedCfgAttr::GatedAttr(attr.span));
}
}

// Visit unremovable (non-optional) expressions -- c.f. `fold_expr` vs `fold_opt_expr`.
fn visit_unremovable_expr(&mut self, expr: &ast::Expr) {
if let Some(attr) = expr.attrs().iter().find(|a| is_cfg(a) || is_test_or_bench(a)) {
let msg = "removing an expression is not supported in this position";
self.diag.diag.span_err(attr.span, msg);
if !self.features.map(|features| features.stmt_expr_attributes).unwrap_or(true) {
emit_feature_err(&self.sess.span_diagnostic,
"stmt_expr_attributes",
attr.span,
GateIssue::Language,
EXPLAIN_STMT_ATTR_SYNTAX);
}
}
}
}

// Support conditional compilation by transforming the AST, stripping out
// any items that do not belong in the current configuration
pub fn strip_unconfigured_items(diagnostic: &Handler, krate: ast::Crate, should_test: bool,
feature_gated_cfgs: &mut Vec<GatedCfgAttr>)
-> ast::Crate
{
let config = &krate.config.clone();
StripUnconfigured::new(config, should_test, diagnostic, feature_gated_cfgs).fold_crate(krate)
pub fn strip_unconfigured_items(mut krate: ast::Crate, sess: &ParseSess, should_test: bool)
-> (ast::Crate, Features) {
let features;
{
let mut strip_unconfigured = StripUnconfigured {
config: &krate.config.clone(),
should_test: should_test,
sess: sess,
features: None,
};

let err_count = sess.span_diagnostic.err_count();
let krate_attrs = strip_unconfigured.process_cfg_attrs(krate.attrs.clone());
features = get_features(&sess.span_diagnostic, &krate_attrs);
if err_count < sess.span_diagnostic.err_count() {
krate.attrs = krate_attrs.clone(); // Avoid reconfiguring malformed `cfg_attr`s
}

strip_unconfigured.features = Some(&features);
krate = strip_unconfigured.fold_crate(krate);
krate.attrs = krate_attrs;
}

(krate, features)
}

impl<'a> fold::Folder for StripUnconfigured<'a> {
Expand Down Expand Up @@ -188,14 +190,19 @@ impl<'a> fold::Folder for StripUnconfigured<'a> {

fn fold_expr(&mut self, expr: P<ast::Expr>) -> P<ast::Expr> {
self.visit_stmt_or_expr_attrs(expr.attrs());

// If an expr is valid to cfg away it will have been removed by the
// outer stmt or expression folder before descending in here.
// Anything else is always required, and thus has to error out
// in case of a cfg attr.
//
// NB: This is intentionally not part of the fold_expr() function
// in order for fold_opt_expr() to be able to avoid this check
self.visit_unremovable_expr(&expr);
if let Some(attr) = expr.attrs().iter().find(|a| is_cfg(a) || is_test_or_bench(a)) {
let msg = "removing an expression is not supported in this position";
self.sess.span_diagnostic.span_err(attr.span, msg);
}

let expr = self.process_cfg_attrs(expr);
fold_expr(self, expr)
}
Expand Down Expand Up @@ -273,22 +280,3 @@ fn is_cfg(attr: &ast::Attribute) -> bool {
fn is_test_or_bench(attr: &ast::Attribute) -> bool {
attr.check_name("test") || attr.check_name("bench")
}

pub trait CfgDiag {
fn emit_error<F>(&mut self, f: F) where F: FnMut(&Handler);
fn flag_gated<F>(&mut self, f: F) where F: FnMut(&mut Vec<GatedCfgAttr>);
}

pub struct CfgDiagReal<'a, 'b> {
pub diag: &'a Handler,
pub feature_gated_cfgs: &'b mut Vec<GatedCfgAttr>,
}

impl<'a, 'b> CfgDiag for CfgDiagReal<'a, 'b> {
fn emit_error<F>(&mut self, mut f: F) where F: FnMut(&Handler) {
f(self.diag)
}
fn flag_gated<F>(&mut self, mut f: F) where F: FnMut(&mut Vec<GatedCfgAttr>) {
f(self.feature_gated_cfgs)
}
}
4 changes: 0 additions & 4 deletions src/libsyntax/ext/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use errors::DiagnosticBuilder;
use ext;
use ext::expand;
use ext::tt::macro_rules;
use feature_gate::GatedCfgAttr;
use parse;
use parse::parser;
use parse::token;
Expand Down Expand Up @@ -556,7 +555,6 @@ pub struct ExtCtxt<'a> {
pub backtrace: ExpnId,
pub ecfg: expand::ExpansionConfig<'a>,
pub crate_root: Option<&'static str>,
pub feature_gated_cfgs: &'a mut Vec<GatedCfgAttr>,
pub loader: &'a mut MacroLoader,

pub mod_path: Vec<ast::Ident> ,
Expand All @@ -573,7 +571,6 @@ pub struct ExtCtxt<'a> {
impl<'a> ExtCtxt<'a> {
pub fn new(parse_sess: &'a parse::ParseSess, cfg: ast::CrateConfig,
ecfg: expand::ExpansionConfig<'a>,
feature_gated_cfgs: &'a mut Vec<GatedCfgAttr>,
loader: &'a mut MacroLoader)
-> ExtCtxt<'a> {
let env = initial_syntax_expander_table(&ecfg);
Expand All @@ -584,7 +581,6 @@ impl<'a> ExtCtxt<'a> {
mod_path: Vec::new(),
ecfg: ecfg,
crate_root: None,
feature_gated_cfgs: feature_gated_cfgs,
exported_macros: Vec::new(),
loader: loader,
syntax_env: env,
Expand Down
Loading