Auto merge of #34002 - jseyfried:strip_tests_in_cfg, r=nrc

Treat `#[test]` like `#[cfg(test)]` in non-test builds

This PR treats `#[test]` like `#[cfg(test)]` in non-test builds. In particular, like `#[cfg(test)]`,
 - `#[test]` nodes are stripped during `cfg` processing, and
 - `#[test]` is disallowed on non-optional expressions.

Closes #33946.
r? @nrc

Author: bors

src/librustc_driver/driver.rs

@@ -582,6 +582,7 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
         sess.track_errors(|| {
             syntax::config::strip_unconfigured_items(sess.diagnostic(),
                                                      krate,
+                                                     sess.opts.test,
                                                      &mut feature_gated_cfgs)
         })
     })?;
@@ -692,6 +693,7 @@ pub fn phase_2_configure_and_expand<'a>(sess: &Session,
             features: Some(&features),
             recursion_limit: sess.recursion_limit.get(),
             trace_mac: sess.opts.debugging_opts.trace_macros,
+            should_test: sess.opts.test,
         };
         let mut loader = macro_import::MacroLoader::new(sess, &cstore, crate_name);
         let mut ecx = syntax::ext::base::ExtCtxt::new(&sess.parse_sess,

src/libsyntax/config.rs

@@ -19,43 +19,37 @@ use ptr::P;
 
 use util::small_vector::SmallVector;
 
-pub trait CfgFolder: fold::Folder {
-    // Check if a node with the given attributes is in this configuration.
-    fn in_cfg(&mut self, attrs: &[ast::Attribute]) -> bool;
-
-    // Update a node before checking if it is in this configuration (used to implement `cfg_attr`).
-    fn process_attrs<T: HasAttrs>(&mut self, node: T) -> T { node }
-
-    // Visit attributes on expression and statements (but not attributes on items in blocks).
-    fn visit_stmt_or_expr_attrs(&mut self, _attrs: &[ast::Attribute]) {}
-
-    // Visit unremovable (non-optional) expressions -- c.f. `fold_expr` vs `fold_opt_expr`.
-    fn visit_unremovable_expr(&mut self, _expr: &ast::Expr) {}
-
-    fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
-        let node = self.process_attrs(node);
-        if self.in_cfg(node.attrs()) { Some(node) } else { None }
-    }
-}
-
-/// A folder that strips out items that do not belong in the current
-/// configuration.
+/// 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,
 }
 
 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 }
+    }
+
+    fn process_cfg_attrs<T: HasAttrs>(&mut self, node: T) -> T {
+        node.map_attrs(|attrs| {
+            attrs.into_iter().filter_map(|attr| self.process_cfg_attr(attr)).collect()
+        })
+    }
+
     fn process_cfg_attr(&mut self, attr: ast::Attribute) -> Option<ast::Attribute> {
         if !attr.check_name("cfg_attr") {
             return Some(attr);
@@ -89,13 +83,15 @@ impl<'a> StripUnconfigured<'a> {
             None
         }
     }
-}
 
-impl<'a> CfgFolder for StripUnconfigured<'a> {
-    // Determine if an item should be translated in the current crate
-    // configuration based on the item's attributes
+    // Determine if a node with the given attributes should be included in this configuation.
     fn in_cfg(&mut self, attrs: &[ast::Attribute]) -> bool {
         attrs.iter().all(|attr| {
+            // When not compiling with --test we should not compile the #[test] functions
+            if !self.should_test && is_test_or_bench(attr) {
+                return false;
+            }
+
             let mis = match attr.node.value.node {
                 ast::MetaItemKind::List(_, ref mis) if is_cfg(&attr) => mis,
                 _ => return true
@@ -112,21 +108,17 @@ impl<'a> CfgFolder for StripUnconfigured<'a> {
         })
     }
 
-    fn process_attrs<T: HasAttrs>(&mut self, node: T) -> T {
-        node.map_attrs(|attrs| {
-            attrs.into_iter().filter_map(|attr| self.process_cfg_attr(attr)).collect()
-        })
-    }
-
+    // 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)) {
+        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);
         }
@@ -135,15 +127,15 @@ impl<'a> CfgFolder for StripUnconfigured<'a> {
 
 // 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,
+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, diagnostic, feature_gated_cfgs).fold_crate(krate)
+    StripUnconfigured::new(config, should_test, diagnostic, feature_gated_cfgs).fold_crate(krate)
 }
 
-impl<T: CfgFolder> fold::Folder for T {
+impl<'a> fold::Folder for StripUnconfigured<'a> {
     fn fold_foreign_mod(&mut self, foreign_mod: ast::ForeignMod) -> ast::ForeignMod {
         ast::ForeignMod {
             abi: foreign_mod.abi,
@@ -204,7 +196,7 @@ impl<T: CfgFolder> fold::Folder for T {
         // 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);
-        let expr = self.process_attrs(expr);
+        let expr = self.process_cfg_attrs(expr);
         fold_expr(self, expr)
     }
 
@@ -256,7 +248,7 @@ impl<T: CfgFolder> fold::Folder for T {
     }
 }
 
-fn fold_expr<F: CfgFolder>(folder: &mut F, expr: P<ast::Expr>) -> P<ast::Expr> {
+fn fold_expr(folder: &mut StripUnconfigured, expr: P<ast::Expr>) -> P<ast::Expr> {
     expr.map(|ast::Expr {id, span, node, attrs}| {
         fold::noop_fold_expr(ast::Expr {
             id: id,
@@ -278,6 +270,10 @@ fn is_cfg(attr: &ast::Attribute) -> bool {
     attr.check_name("cfg")
 }
 
+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>);

src/libsyntax/ext/expand.rs

@@ -1001,6 +1001,7 @@ 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)
     }
@@ -1106,6 +1107,7 @@ pub struct ExpansionConfig<'feat> {
     pub features: Option<&'feat Features>,
     pub recursion_limit: usize,
     pub trace_mac: bool,
+    pub should_test: bool, // If false, strip `#[test]` nodes
 }
 
 macro_rules! feature_tests {
@@ -1128,6 +1130,7 @@ impl<'feat> ExpansionConfig<'feat> {
             features: None,
             recursion_limit: 64,
             trace_mac: false,
+            should_test: false,
         }
     }
 

src/libsyntax/test.rs

@@ -81,7 +81,7 @@ pub fn modify_for_testing(sess: &ParseSess,
     if should_test {
         generate_test_harness(sess, reexport_test_harness_main, krate, span_diagnostic)
     } else {
-        strip_test_functions(krate)
+        krate
     }
 }
 
@@ -306,19 +306,6 @@ fn generate_test_harness(sess: &ParseSess,
     return res;
 }
 
-fn strip_test_functions(krate: ast::Crate) -> ast::Crate {
-    // When not compiling with --test we should not compile the
-    // #[test] functions
-    struct StripTests;
-    impl config::CfgFolder for StripTests {
-        fn in_cfg(&mut self, attrs: &[ast::Attribute]) -> bool {
-            !attr::contains_name(attrs, "test") && !attr::contains_name(attrs, "bench")
-        }
-    }
-
-    StripTests.fold_crate(krate)
-}
-
 /// Craft a span that will be ignored by the stability lint's
 /// call to codemap's is_internal check.
 /// The expanded code calls some unstable functions in the test crate.

src/test/compile-fail/cfg-non-opt-expr.rs

@@ -17,4 +17,6 @@ fn main() {
     //~^ ERROR removing an expression is not supported in this position
     let _ = [1, 2, 3][#[cfg(unset)] 1];
     //~^ ERROR removing an expression is not supported in this position
+    let _ = #[test] ();
+    //~^ ERROR removing an expression is not supported in this position
 }