Temporarily prohibit proc macro attributes placed after derives

petrochenkov · petrochenkov · commit 229df02c0b85 · 2018-09-16T19:51:41.000+03:00
... and also proc macro attributes used together with test/bench.
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
@@ -25,7 +25,7 @@ use syntax::attr;
 use syntax::errors::DiagnosticBuilder;
 use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator};
 use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
-use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
+use syntax::ext::expand::{AstFragment, Invocation, InvocationKind, TogetherWith};
 use syntax::ext::hygiene::{self, Mark};
 use syntax::ext::tt::macro_rules;
 use syntax::feature_gate::{self, feature_err, emit_feature_err, is_builtin_attr_name, GateIssue};
@@ -341,21 +341,30 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
 
     fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: Mark, force: bool)
                                 -> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
-        let (path, kind, derives_in_scope) = match invoc.kind {
+        let (path, kind, derives_in_scope, together_with) = match invoc.kind {
             InvocationKind::Attr { attr: None, .. } =>
                 return Ok(None),
-            InvocationKind::Attr { attr: Some(ref attr), ref traits, .. } =>
-                (&attr.path, MacroKind::Attr, traits.clone()),
+            InvocationKind::Attr { attr: Some(ref attr), ref traits, together_with, .. } =>
+                (&attr.path, MacroKind::Attr, traits.clone(), together_with),
             InvocationKind::Bang { ref mac, .. } =>
-                (&mac.node.path, MacroKind::Bang, Vec::new()),
+                (&mac.node.path, MacroKind::Bang, Vec::new(), TogetherWith::None),
             InvocationKind::Derive { ref path, .. } =>
-                (path, MacroKind::Derive, Vec::new()),
+                (path, MacroKind::Derive, Vec::new(), TogetherWith::None),
         };
 
         let parent_scope = self.invoc_parent_scope(invoc_id, derives_in_scope);
         let (def, ext) = self.resolve_macro_to_def(path, kind, &parent_scope, force)?;
 
         if let Def::Macro(def_id, _) = def {
+            match together_with {
+                TogetherWith::Derive =>
+                    self.session.span_err(invoc.span(),
+                        "macro attributes must be placed before `#[derive]`"),
+                TogetherWith::TestBench if !self.session.features_untracked().plugin =>
+                    self.session.span_err(invoc.span(),
+                        "macro attributes cannot be used together with `#[test]` or `#[bench]`"),
+                _ => {}
+            }
             self.macro_defs.insert(invoc.expansion_data.mark, def_id);
             let normal_module_def_id =
                 self.macro_def_scope(invoc.expansion_data.mark).normal_ancestor_id;
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
@@ -216,6 +216,14 @@ pub struct Invocation {
     pub expansion_data: ExpansionData,
 }
 
+// Needed for feature-gating attributes used after derives or together with test/bench
+#[derive(Clone, Copy, PartialEq)]
+pub enum TogetherWith {
+    None,
+    Derive,
+    TestBench,
+}
+
 pub enum InvocationKind {
     Bang {
         mac: ast::Mac,
@@ -226,6 +234,7 @@ pub enum InvocationKind {
         attr: Option<ast::Attribute>,
         traits: Vec<Path>,
         item: Annotatable,
+        together_with: TogetherWith,
     },
     Derive {
         path: Path,
@@ -353,7 +362,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                     let dummy = invoc.fragment_kind.dummy(invoc.span()).unwrap();
                     let fragment = self.expand_invoc(invoc, &*ext).unwrap_or(dummy);
                     self.collect_invocations(fragment, &[])
-                } else if let InvocationKind::Attr { attr: None, traits, item } = invoc.kind {
+                } else if let InvocationKind::Attr { attr: None, traits, item, .. } = invoc.kind {
                     if !item.derive_allowed() {
                         let attr = attr::find_by_name(item.attrs(), "derive")
                             .expect("`derive` attribute should exist");
@@ -1069,14 +1078,23 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                     attr: Option<ast::Attribute>,
                     traits: Vec<Path>,
                     item: Annotatable,
-                    kind: AstFragmentKind)
+                    kind: AstFragmentKind,
+                    together_with: TogetherWith)
                     -> AstFragment {
-        self.collect(kind, InvocationKind::Attr { attr, traits, item })
+        self.collect(kind, InvocationKind::Attr { attr, traits, item, together_with })
     }
 
-    fn find_attr_invoc(&self, attrs: &mut Vec<ast::Attribute>) -> Option<ast::Attribute> {
+    fn find_attr_invoc(&self, attrs: &mut Vec<ast::Attribute>, together_with: &mut TogetherWith)
+                       -> Option<ast::Attribute> {
         let attr = attrs.iter()
-                        .position(|a| !attr::is_known(a) && !is_builtin_attr(a))
+                        .position(|a| {
+                            if a.path == "derive" {
+                                *together_with = TogetherWith::Derive
+                            } else if a.path == "rustc_test_marker2" {
+                                *together_with = TogetherWith::TestBench
+                            }
+                            !attr::is_known(a) && !is_builtin_attr(a)
+                        })
                         .map(|i| attrs.remove(i));
         if let Some(attr) = &attr {
             if !self.cx.ecfg.enable_custom_inner_attributes() &&
@@ -1086,14 +1104,19 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                                  "non-builtin inner attributes are unstable");
             }
         }
+        if together_with == &TogetherWith::None &&
+           attrs.iter().any(|a| a.path == "rustc_test_marker2") {
+            *together_with = TogetherWith::TestBench;
+        }
         attr
     }
 
     /// If `item` is an attr invocation, remove and return the macro attribute and derive traits.
-    fn classify_item<T>(&mut self, mut item: T) -> (Option<ast::Attribute>, Vec<Path>, T)
+    fn classify_item<T>(&mut self, mut item: T)
+                        -> (Option<ast::Attribute>, Vec<Path>, T, TogetherWith)
         where T: HasAttrs,
     {
-        let (mut attr, mut traits) = (None, Vec::new());
+        let (mut attr, mut traits, mut together_with) = (None, Vec::new(), TogetherWith::None);
 
         item = item.map_attrs(|mut attrs| {
             if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
@@ -1102,19 +1125,20 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 return attrs;
             }
 
-            attr = self.find_attr_invoc(&mut attrs);
+            attr = self.find_attr_invoc(&mut attrs, &mut together_with);
             traits = collect_derives(&mut self.cx, &mut attrs);
             attrs
         });
 
-        (attr, traits, item)
+        (attr, traits, item, together_with)
     }
 
     /// Alternative of `classify_item()` that ignores `#[derive]` so invocations fallthrough
     /// to the unused-attributes lint (making it an error on statements and expressions
     /// is a breaking change)
-    fn classify_nonitem<T: HasAttrs>(&mut self, mut item: T) -> (Option<ast::Attribute>, T) {
-        let mut attr = None;
+    fn classify_nonitem<T: HasAttrs>(&mut self, mut item: T)
+                                     -> (Option<ast::Attribute>, T, TogetherWith) {
+        let (mut attr, mut together_with) = (None, TogetherWith::None);
 
         item = item.map_attrs(|mut attrs| {
             if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
@@ -1123,11 +1147,11 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
                 return attrs;
             }
 
-            attr = self.find_attr_invoc(&mut attrs);
+            attr = self.find_attr_invoc(&mut attrs, &mut together_with);
             attrs
         });
 
-        (attr, item)
+        (attr, item, together_with)
     }
 
     fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
@@ -1166,7 +1190,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
         expr.node = self.cfg.configure_expr_kind(expr.node);
 
         // ignore derives so they remain unused
-        let (attr, expr) = self.classify_nonitem(expr);
+        let (attr, expr, together_with) = self.classify_nonitem(expr);
 
         if attr.is_some() {
             // collect the invoc regardless of whether or not attributes are permitted here
@@ -1175,7 +1199,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
 
             // AstFragmentKind::Expr requires the macro to emit an expression
             return self.collect_attr(attr, vec![], Annotatable::Expr(P(expr)),
-                                     AstFragmentKind::Expr).make_expr();
+                                     AstFragmentKind::Expr, together_with).make_expr();
         }
 
         if let ast::ExprKind::Mac(mac) = expr.node {
@@ -1191,14 +1215,13 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
         expr.node = self.cfg.configure_expr_kind(expr.node);
 
         // ignore derives so they remain unused
-        let (attr, expr) = self.classify_nonitem(expr);
+        let (attr, expr, together_with) = self.classify_nonitem(expr);
 
         if attr.is_some() {
             attr.as_ref().map(|a| self.cfg.maybe_emit_expr_attr_err(a));
 
             return self.collect_attr(attr, vec![], Annotatable::Expr(P(expr)),
-                                     AstFragmentKind::OptExpr)
-                .make_opt_expr();
+                                     AstFragmentKind::OptExpr, together_with).make_opt_expr();
         }
 
         if let ast::ExprKind::Mac(mac) = expr.node {
@@ -1230,19 +1253,18 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
 
         // we'll expand attributes on expressions separately
         if !stmt.is_expr() {
-            let (attr, derives, stmt_) = if stmt.is_item() {
+            let (attr, derives, stmt_, together_with) = if stmt.is_item() {
                 self.classify_item(stmt)
             } else {
                 // ignore derives on non-item statements so it falls through
                 // to the unused-attributes lint
-                let (attr, stmt) = self.classify_nonitem(stmt);
-                (attr, vec![], stmt)
+                let (attr, stmt, together_with) = self.classify_nonitem(stmt);
+                (attr, vec![], stmt, together_with)
             };
 
             if attr.is_some() || !derives.is_empty() {
-                return self.collect_attr(attr, derives,
-                                         Annotatable::Stmt(P(stmt_)), AstFragmentKind::Stmts)
-                    .make_stmts();
+                return self.collect_attr(attr, derives, Annotatable::Stmt(P(stmt_)),
+                                         AstFragmentKind::Stmts, together_with).make_stmts();
             }
 
             stmt = stmt_;
@@ -1284,10 +1306,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
     fn fold_item(&mut self, item: P<ast::Item>) -> OneVector<P<ast::Item>> {
         let item = configure!(self, item);
 
-        let (attr, traits, item) = self.classify_item(item);
+        let (attr, traits, item, together_with) = self.classify_item(item);
         if attr.is_some() || !traits.is_empty() {
-            let item = Annotatable::Item(item);
-            return self.collect_attr(attr, traits, item, AstFragmentKind::Items).make_items();
+            return self.collect_attr(attr, traits, Annotatable::Item(item),
+                                     AstFragmentKind::Items, together_with).make_items();
         }
 
         match item.node {
@@ -1359,11 +1381,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
     fn fold_trait_item(&mut self, item: ast::TraitItem) -> OneVector<ast::TraitItem> {
         let item = configure!(self, item);
 
-        let (attr, traits, item) = self.classify_item(item);
+        let (attr, traits, item, together_with) = self.classify_item(item);
         if attr.is_some() || !traits.is_empty() {
-            let item = Annotatable::TraitItem(P(item));
-            return self.collect_attr(attr, traits, item, AstFragmentKind::TraitItems)
-                .make_trait_items()
+            return self.collect_attr(attr, traits, Annotatable::TraitItem(P(item)),
+                                     AstFragmentKind::TraitItems, together_with).make_trait_items()
         }
 
         match item.node {
@@ -1379,11 +1400,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
     fn fold_impl_item(&mut self, item: ast::ImplItem) -> OneVector<ast::ImplItem> {
         let item = configure!(self, item);
 
-        let (attr, traits, item) = self.classify_item(item);
+        let (attr, traits, item, together_with) = self.classify_item(item);
         if attr.is_some() || !traits.is_empty() {
-            let item = Annotatable::ImplItem(P(item));
-            return self.collect_attr(attr, traits, item, AstFragmentKind::ImplItems)
-                .make_impl_items();
+            return self.collect_attr(attr, traits, Annotatable::ImplItem(P(item)),
+                                     AstFragmentKind::ImplItems, together_with).make_impl_items();
         }
 
         match item.node {
@@ -1414,12 +1434,12 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
 
     fn fold_foreign_item(&mut self,
                          foreign_item: ast::ForeignItem) -> OneVector<ast::ForeignItem> {
-        let (attr, traits, foreign_item) = self.classify_item(foreign_item);
+        let (attr, traits, foreign_item, together_with) = self.classify_item(foreign_item);
 
         if attr.is_some() || !traits.is_empty() {
-            let item = Annotatable::ForeignItem(P(foreign_item));
-            return self.collect_attr(attr, traits, item, AstFragmentKind::ForeignItems)
-                .make_foreign_items();
+            return self.collect_attr(attr, traits, Annotatable::ForeignItem(P(foreign_item)),
+                                     AstFragmentKind::ForeignItems, together_with)
+                                     .make_foreign_items();
         }
 
         if let ast::ForeignItemKind::Macro(mac) = foreign_item.node {
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
@@ -970,6 +970,10 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
                                      "the `#[rustc_test_marker]` attribute \
                                       is used internally to track tests",
                                      cfg_fn!(rustc_attrs))),
+    ("rustc_test_marker2", Normal, Gated(Stability::Unstable,
+                                         "rustc_attrs",
+                                         "temporarily used by rustc to report some errors",
+                                         cfg_fn!(rustc_attrs))),
     ("rustc_transparent_macro", Whitelisted, Gated(Stability::Unstable,
                                                    "rustc_attrs",
                                                    "used internally for testing macro hygiene",
diff --git a/src/libsyntax/ptr.rs b/src/libsyntax/ptr.rs
@@ -38,7 +38,7 @@
 
 use std::fmt::{self, Display, Debug};
 use std::iter::FromIterator;
-use std::ops::Deref;
+use std::ops::{Deref, DerefMut};
 use std::{mem, ptr, slice, vec};
 
 use serialize::{Encodable, Decodable, Encoder, Decoder};
@@ -103,6 +103,12 @@ impl<T: ?Sized> Deref for P<T> {
     }
 }
 
+impl<T: ?Sized> DerefMut for P<T> {
+    fn deref_mut(&mut self) -> &mut T {
+        &mut self.ptr
+    }
+}
+
 impl<T: 'static + Clone> Clone for P<T> {
     fn clone(&self) -> P<T> {
         P((**self).clone())
diff --git a/src/libsyntax_ext/test.rs b/src/libsyntax_ext/test.rs
@@ -49,7 +49,7 @@ pub fn expand_test_or_bench(
     // If we're not in test configuration, remove the annotated item
     if !cx.ecfg.should_test { return vec![]; }
 
-    let item =
+    let mut item =
         if let Annotatable::Item(i) = item { i }
         else {
             cx.parse_sess.span_diagnostic.span_fatal(item.span(),
@@ -192,6 +192,12 @@ pub fn expand_test_or_bench(
 
     debug!("Synthetic test item:\n{}\n", pprust::item_to_string(&test_const));
 
+    // Temporarily add another marker to the original item for error reporting
+    let marker2 = cx.attribute(
+        attr_sp, cx.meta_word(attr_sp, Symbol::intern("rustc_test_marker2"))
+    );
+    item.attrs.push(marker2);
+
     vec![
         // Access to libtest under a gensymed name
         Annotatable::Item(test_extern),
diff --git a/src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs b/src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs
@@ -16,12 +16,12 @@
 #[macro_use]
 extern crate derive_b;
 
-#[derive(B)]
 #[B] //~ ERROR `B` is a derive mode
 #[C]
 #[B(D)]
 #[B(E = "foo")]
 #[B(arbitrary tokens)]
+#[derive(B)]
 struct B;
 
 fn main() {}
diff --git a/src/test/ui-fulldeps/attribute-order-restricted.rs b/src/test/ui-fulldeps/attribute-order-restricted.rs
@@ -0,0 +1,32 @@
+// aux-build:attr_proc_macro.rs
+// compile-flags:--test
+
+#![feature(test)]
+
+extern crate test;
+extern crate attr_proc_macro;
+use attr_proc_macro::*;
+
+#[attr_proc_macro] // OK
+#[derive(Clone)]
+struct Before;
+
+#[derive(Clone)]
+#[attr_proc_macro] //~ ERROR macro attributes must be placed before `#[derive]`
+struct After;
+
+#[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]`
+#[test]
+fn test_before() {}
+
+#[test]
+#[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]`
+fn test_after() {}
+
+#[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]`
+#[bench]
+fn bench_before(b: &mut test::Bencher) {}
+
+#[bench]
+#[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]`
+fn bench_after(b: &mut test::Bencher) {}
diff --git a/src/test/ui-fulldeps/attribute-order-restricted.stderr b/src/test/ui-fulldeps/attribute-order-restricted.stderr
diff --git a/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs b/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs
diff --git a/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr b/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr
