Mirrors/rust
1
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2026-05-15 20:45:45 +03:00
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #54277 - petrochenkov:afterder, r=alexcrichton

Browse Source 
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
This commit is contained in:
bors
2018-09-17 18:13:26 +00:00
parent 186fe09143 229df02c0b
commit 354a29a5f1
10 changed files with 160 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/librustc_resolve/macros.rs
+15 -6
View File
@@ -24,7 +24,7 @@
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 @@ fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>, allow_deri
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;
src/libsyntax/ext/expand.rs
+59 -39
View File
@@ -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 @@ fn expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment {
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 @@ fn collect_attr(&mut self,
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 @@ fn find_attr_invoc(&self, attrs: &mut Vec<ast::Attribute>) -> Option<ast::Attrib
"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 @@ fn classify_item<T>(&mut self, mut item: T) -> (Option<ast::Attribute>, Vec<Path
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 @@ fn classify_nonitem<T: HasAttrs>(&mut self, mut item: T) -> (Option<ast::Attribu
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 @@ fn fold_expr(&mut self, expr: P<ast::Expr>) -> P<ast::Expr> {
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 @@ fn fold_expr(&mut self, expr: P<ast::Expr>) -> P<ast::Expr> {
// 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 @@ fn fold_opt_expr(&mut self, expr: P<ast::Expr>) -> Option<P<ast::Expr>> {
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 @@ fn fold_stmt(&mut self, stmt: ast::Stmt) -> OneVector<ast::Stmt> {
// 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 @@ fn fold_block(&mut self, block: P<Block>) -> P<Block> {
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 @@ fn fold_item(&mut self, item: P<ast::Item>) -> OneVector<P<ast::Item>> {
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 @@ fn fold_trait_item(&mut self, item: ast::TraitItem) -> OneVector<ast::TraitItem>
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 @@ fn fold_foreign_mod(&mut self, foreign_mod: ast::ForeignMod) -> ast::ForeignMod
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 {
src/libsyntax/feature_gate.rs
+4
View File
@@ -970,6 +970,10 @@ pub fn is_builtin_attr(attr: &ast::Attribute) -> bool {
"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",
src/libsyntax/ptr.rs
+7 -1
View File
@@ -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 @@ fn deref(&self) -> &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())
src/libsyntax_ext/test.rs
+7 -1
View File
@@ -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),
src/test/compile-fail-fulldeps/proc-macro/proc-macro-attributes.rs
+1 -1
View File
@@ -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() {}
src/test/ui-fulldeps/attribute-order-restricted.rs
+32
View File
@@ -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) {}
src/test/ui-fulldeps/attribute-order-restricted.stderr
+32
View File
@@ -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
src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.rs
+1 -1
View File
@@ -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() {}
src/test/ui-fulldeps/proc-macro/derive-helper-shadowing.stderr
+2 -2
View File
@@ -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)]
| ^^^^^^^