Simplify gated cfg checking

6 files changed, 95 insertions, 169 deletions

diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs
index c3c3deea187..e36e15802f0 100644
--- a/src/libsyntax/attr.rs
+++ b/src/libsyntax/attr.rs
@@ -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};
@@ -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)
         }
     }
diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs
index efd92ab3240..0e5d6841c82 100644
--- a/src/libsyntax/config.rs
+++ b/src/libsyntax/config.rs
@@ -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 }
@@ -59,7 +47,7 @@ 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;
             }
         };
@@ -67,12 +55,12 @@ impl<'a> StripUnconfigured<'a> {
             (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,
@@ -98,13 +86,11 @@ 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)
         })
     }
 
@@ -112,27 +98,43 @@ impl<'a> StripUnconfigured<'a> {
     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> {
@@ -188,6 +190,7 @@ 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
@@ -195,7 +198,11 @@ impl<'a> fold::Folder for StripUnconfigured<'a> {
         //
         // 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)
     }
@@ -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)
-    }
-}
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index dba7d987d9f..4f700e9170f 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -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;
@@ -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> ,
@@ -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);
@@ -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,
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 4631ffcf0ff..ed419d94ee4 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -998,10 +998,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
     }
 
     fn strip_unconfigured(&mut self) -> StripUnconfigured {
-        StripUnconfigured::new(&self.cx.cfg,
-                               self.cx.ecfg.should_test,
-                               &self.cx.parse_sess.span_diagnostic,
-                               self.cx.feature_gated_cfgs)
+        StripUnconfigured {
+            config: &self.cx.cfg,
+            should_test: self.cx.ecfg.should_test,
+            sess: self.cx.parse_sess,
+            features: self.cx.ecfg.features,
+        }
     }
 
     fn load_macros<T: MacroGenerable>(&mut self, node: &T) {
@@ -1331,8 +1333,8 @@ mod tests {
             src,
             Vec::new(), &sess).unwrap();
         // should fail:
-        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
-        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        let mut loader = DummyMacroLoader;
+        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut loader);
         expand_crate(ecx, vec![], crate_ast);
     }
 
@@ -1346,8 +1348,8 @@ mod tests {
             "<test>".to_string(),
             src,
             Vec::new(), &sess).unwrap();
-        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
-        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        let mut loader = DummyMacroLoader;
+        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut loader);
         expand_crate(ecx, vec![], crate_ast);
     }
 
@@ -1360,8 +1362,8 @@ mod tests {
             "<test>".to_string(),
             src,
             Vec::new(), &sess).unwrap();
-        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
-        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        let mut loader = DummyMacroLoader;
+        let ecx = ExtCtxt::new(&sess, vec![], test_ecfg(), &mut loader);
         expand_crate(ecx, vec![], crate_ast);
     }
 
@@ -1369,8 +1371,8 @@ mod tests {
         let ps = parse::ParseSess::new();
         let crate_ast = panictry!(string_to_parser(&ps, crate_str).parse_crate_mod());
         // the cfg argument actually does matter, here...
-        let (mut gated_cfgs, mut loader) = (vec![], DummyMacroLoader);
-        let ecx = ExtCtxt::new(&ps, vec![], test_ecfg(), &mut gated_cfgs, &mut loader);
+        let mut loader = DummyMacroLoader;
+        let ecx = ExtCtxt::new(&ps, vec![], test_ecfg(), &mut loader);
         expand_crate(ecx, vec![], crate_ast).0
     }
 
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
index 86c4a33896d..550eb0a56d9 100644
--- a/src/libsyntax/feature_gate.rs
+++ b/src/libsyntax/feature_gate.rs
@@ -34,10 +34,10 @@ use codemap::{CodeMap, Span};
 use errors::Handler;
 use visit;
 use visit::{FnKind, Visitor};
+use parse::ParseSess;
 use parse::token::InternedString;
 
 use std::ascii::AsciiExt;
-use std::cmp;
 
 macro_rules! setter {
     ($field: ident) => {{
@@ -604,59 +604,11 @@ const GATED_CFGS: &'static [(&'static str, &'static str, fn(&Features) -> bool)]
 ];
 
 #[derive(Debug, Eq, PartialEq)]
-pub enum GatedCfgAttr {
-    GatedCfg(GatedCfg),
-    GatedAttr(Span),
-}
-
-#[derive(Debug, Eq, PartialEq)]
 pub struct GatedCfg {
     span: Span,
     index: usize,
 }
 
-impl Ord for GatedCfgAttr {
-    fn cmp(&self, other: &GatedCfgAttr) -> cmp::Ordering {
-        let to_tup = |s: &GatedCfgAttr| match *s {
-            GatedCfgAttr::GatedCfg(ref gated_cfg) => {
-                (gated_cfg.span.lo.0, gated_cfg.span.hi.0, gated_cfg.index)
-            }
-            GatedCfgAttr::GatedAttr(ref span) => {
-                (span.lo.0, span.hi.0, GATED_CFGS.len())
-            }
-        };
-        to_tup(self).cmp(&to_tup(other))
-    }
-}
-
-impl PartialOrd for GatedCfgAttr {
-    fn partial_cmp(&self, other: &GatedCfgAttr) -> Option<cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl GatedCfgAttr {
-    pub fn check_and_emit(&self,
-                          diagnostic: &Handler,
-                          features: &Features,
-                          codemap: &CodeMap) {
-        match *self {
-            GatedCfgAttr::GatedCfg(ref cfg) => {
-                cfg.check_and_emit(diagnostic, features, codemap);
-            }
-            GatedCfgAttr::GatedAttr(span) => {
-                if !features.stmt_expr_attributes {
-                    emit_feature_err(diagnostic,
-                                     "stmt_expr_attributes",
-                                     span,
-                                     GateIssue::Language,
-                                     EXPLAIN_STMT_ATTR_SYNTAX);
-                }
-            }
-        }
-    }
-}
-
 impl GatedCfg {
     pub fn gate(cfg: &ast::MetaItem) -> Option<GatedCfg> {
         let name = cfg.name();
@@ -669,12 +621,11 @@ impl GatedCfg {
                       }
                   })
     }
-    fn check_and_emit(&self,
-                      diagnostic: &Handler,
-                      features: &Features,
-                      codemap: &CodeMap) {
+
+    pub fn check_and_emit(&self, sess: &ParseSess, features: &Features) {
         let (cfg, feature, has_feature) = GATED_CFGS[self.index];
-        if !has_feature(features) && !codemap.span_allows_unstable(self.span) {
+        if !has_feature(features) && !sess.codemap().span_allows_unstable(self.span) {
+            let diagnostic = &sess.span_diagnostic;
             let explain = format!("`cfg({})` is experimental and subject to change", cfg);
             emit_feature_err(diagnostic, feature, self.span, GateIssue::Language, &explain);
         }
@@ -810,7 +761,7 @@ pub fn emit_feature_err(diag: &Handler, feature: &str, span: Span, issue: GateIs
 const EXPLAIN_BOX_SYNTAX: &'static str =
     "box expression syntax is experimental; you can call `Box::new` instead.";
 
-const EXPLAIN_STMT_ATTR_SYNTAX: &'static str =
+pub const EXPLAIN_STMT_ATTR_SYNTAX: &'static str =
     "attributes on non-item statements and expressions are experimental.";
 
 pub const EXPLAIN_ASM: &'static str =
@@ -1142,10 +1093,10 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
     }
 }
 
-pub fn get_features(span_handler: &Handler, krate: &ast::Crate) -> Features {
+pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute]) -> Features {
     let mut features = Features::new();
 
-    for attr in &krate.attrs {
+    for attr in krate_attrs {
         if !attr.check_name("feature") {
             continue
         }
@@ -1188,21 +1139,19 @@ pub fn get_features(span_handler: &Handler, krate: &ast::Crate) -> Features {
     features
 }
 
-pub fn check_crate(cm: &CodeMap, span_handler: &Handler, krate: &ast::Crate,
+pub fn check_crate(krate: &ast::Crate,
+                   sess: &ParseSess,
+                   features: &Features,
                    plugin_attributes: &[(String, AttributeType)],
-                   unstable: UnstableFeatures) -> Features {
-    maybe_stage_features(span_handler, krate, unstable);
-    let features = get_features(span_handler, krate);
-    {
-        let ctx = Context {
-            features: &features,
-            span_handler: span_handler,
-            cm: cm,
-            plugin_attributes: plugin_attributes,
-        };
-        visit::walk_crate(&mut PostExpansionVisitor { context: &ctx }, krate);
-    }
-    features
+                   unstable: UnstableFeatures) {
+    maybe_stage_features(&sess.span_diagnostic, krate, unstable);
+    let ctx = Context {
+        features: features,
+        span_handler: &sess.span_diagnostic,
+        cm: sess.codemap(),
+        plugin_attributes: plugin_attributes,
+    };
+    visit::walk_crate(&mut PostExpansionVisitor { context: &ctx }, krate);
 }
 
 #[derive(Clone, Copy)]
diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs
index 6e29505f00a..ca6ed76d549 100644
--- a/src/libsyntax/test.rs
+++ b/src/libsyntax/test.rs
@@ -270,14 +270,12 @@ fn generate_test_harness(sess: &ParseSess,
     let mut cleaner = EntryPointCleaner { depth: 0 };
     let krate = cleaner.fold_crate(krate);
 
-    let mut feature_gated_cfgs = vec![];
     let mut loader = DummyMacroLoader;
     let mut cx: TestCtxt = TestCtxt {
         sess: sess,
         span_diagnostic: sd,
         ext_cx: ExtCtxt::new(sess, vec![],
                              ExpansionConfig::default("test".to_string()),
-                             &mut feature_gated_cfgs,
                              &mut loader),
         path: Vec::new(),
         testfns: Vec::new(),