diff options
| author | bors <bors@rust-lang.org> | 2018-09-17 18:13:26 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2018-09-17 18:13:26 +0000 |
| commit | 354a29a5f1feb55bd799f874786e013beddfd645 (patch) | |
| tree | 5d8b8dc7b289b1af380df5fdbe86751fe9f42693 /src | |
| parent | 186fe091434b4c20c160b8098a56bb6a841bf6b1 (diff) | |
| parent | 229df02c0b85c33465d7191a6e085e964e56c1c9 (diff) | |
| download | rust-354a29a5f1feb55bd799f874786e013beddfd645.tar.gz rust-354a29a5f1feb55bd799f874786e013beddfd645.zip | |
Auto merge of #54277 - petrochenkov:afterder, r=alexcrichton
Temporarily prohibit proc macro attributes placed after derives ... and also proc macro attributes used together with `#[test]`/`#[bench]`. Addresses item 6 from https://github.com/rust-lang/rust/pull/50911#issuecomment-411605393. The end goal is straightforward predictable left-to-right expansion order for attributes. Right now derives are expanded last regardless of their relative ordering with macro attributes and right now it's simpler to temporarily prohibit macro attributes placed after derives than changing the expansion order. I'm not sure whether the new beta is already released or not, but if it's released, then this patch needs to be backported, so the solution needs to be minimal. How to fix broken code (derives): - Move macro attributes above derives. This won't change expansion order, they are expanded before derives anyway. Using attribute macros on same items with `#[test]` and `#[bench]` is prohibited for similar expansion order reasons, but this one is going to be reverted much sooner than restrictions on derives. How to fix broken code (test/bench): - Enable `#![feature(plugin)]` (don't ask why). r? @ghost
Diffstat (limited to 'src')
| -rw-r--r-- | src/librustc_resolve/macros.rs | 21 | ||||
| -rw-r--r-- | src/libsyntax/ext/expand.rs | 98 | ||||
| -rw-r--r-- | src/libsyntax/feature_gate.rs | 4 | ||||
| -rw-r--r-- | src/libsyntax/ptr.rs | 8 | ||||
| -rw-r--r-- | src/libsyntax_ext/test.rs | 8 | ||||
| -rw-r--r-- | src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs | 2 | ||||
| -rw-r--r-- | src/test/ui-fulldeps/attribute-order-restricted.rs | 32 | ||||
| -rw-r--r-- | src/test/ui-fulldeps/attribute-order-restricted.stderr | 32 | ||||
| -rw-r--r-- | src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs | 2 | ||||
| -rw-r--r-- | src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr | 4 |
10 files changed, 160 insertions, 51 deletions
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index b9be52d8e05..cb561b2597b 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -24,7 +24,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}; @@ -332,21 +332,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 index 40903e8ad6c..33d0e76ca48 100644 --- 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 index 7266d807d3b..060faa9856d 100644 --- 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 index b86d19ba3ce..bb47d9b535b 100644 --- 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 index be3485cfa7c..8ddfb1d9cba 100644 --- 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 index 215d51c2270..83bbb7c13c4 100644 --- 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 new file mode 100644 index 00000000000..553cd86e620 --- /dev/null +++ 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 new file mode 100644 index 00000000000..841fc630b22 --- /dev/null +++ b/src/test/ui-fulldeps/attribute-order-restricted.stderr @@ -0,0 +1,32 @@ +error: macro attributes must be placed before `#[derive]` + --> $DIR/attribute-order-restricted.rs:15:1 + | +LL | #[attr_proc_macro] //~ ERROR macro attributes must be placed before `#[derive]` + | ^^^^^^^^^^^^^^^^^^ + +error: macro attributes cannot be used together with `#[test]` or `#[bench]` + --> $DIR/attribute-order-restricted.rs:18:1 + | +LL | #[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]` + | ^^^^^^^^^^^^^^^^^^ + +error: macro attributes cannot be used together with `#[test]` or `#[bench]` + --> $DIR/attribute-order-restricted.rs:23:1 + | +LL | #[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]` + | ^^^^^^^^^^^^^^^^^^ + +error: macro attributes cannot be used together with `#[test]` or `#[bench]` + --> $DIR/attribute-order-restricted.rs:26:1 + | +LL | #[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]` + | ^^^^^^^^^^^^^^^^^^ + +error: macro attributes cannot be used together with `#[test]` or `#[bench]` + --> $DIR/attribute-order-restricted.rs:31:1 + | +LL | #[attr_proc_macro] //~ ERROR macro attributes cannot be used together with `#[test]` or `#[bench]` + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs b/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs index c2357d501ee..aa9eae0ba31 100644 --- a/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs +++ b/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs @@ -3,8 +3,8 @@ extern crate derive_helper_shadowing; use derive_helper_shadowing::*; -#[derive(MyTrait)] #[my_attr] //~ ERROR `my_attr` is ambiguous +#[derive(MyTrait)] struct S; fn main() {} diff --git a/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr b/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr index d597b577bb7..cdfecb3d101 100644 --- a/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr +++ b/src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr @@ -1,5 +1,5 @@ error[E0659]: `my_attr` is ambiguous - --> $DIR/derive-helper-shadowing.rs:7:3 + --> $DIR/derive-helper-shadowing.rs:6:3 | LL | #[my_attr] //~ ERROR `my_attr` is ambiguous | ^^^^^^^ ambiguous name @@ -10,7 +10,7 @@ note: `my_attr` could refer to the name imported here LL | use derive_helper_shadowing::*; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ note: `my_attr` could also refer to the name defined here - --> $DIR/derive-helper-shadowing.rs:6:10 + --> $DIR/derive-helper-shadowing.rs:7:10 | LL | #[derive(MyTrait)] | ^^^^^^^ |
