Skip to content

Commit

Permalink
expansion: Remove restriction on use of macro attributes with test/bench
Browse files Browse the repository at this point in the history
The restrictions were introduced in #54277 and no longer necessary now because legacy plugins are now expanded in usual left-to-right order
  • Loading branch information
petrochenkov committed Oct 5, 2018
1 parent 60a1d4e commit 8994c6d
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 114 deletions.
23 changes: 9 additions & 14 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use syntax::attr;
use syntax::errors::DiagnosticBuilder;
use syntax::ext::base::{self, Determinacy};
use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
use syntax::ext::expand::{AstFragment, Invocation, InvocationKind, TogetherWith};
use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
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};
Expand Down Expand Up @@ -313,29 +313,24 @@ 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, together_with) = match invoc.kind {
let (path, kind, derives_in_scope, after_derive) = match invoc.kind {
InvocationKind::Attr { attr: None, .. } =>
return Ok(None),
InvocationKind::Attr { attr: Some(ref attr), ref traits, together_with, .. } =>
(&attr.path, MacroKind::Attr, traits.clone(), together_with),
InvocationKind::Attr { attr: Some(ref attr), ref traits, after_derive, .. } =>
(&attr.path, MacroKind::Attr, traits.clone(), after_derive),
InvocationKind::Bang { ref mac, .. } =>
(&mac.node.path, MacroKind::Bang, Vec::new(), TogetherWith::None),
(&mac.node.path, MacroKind::Bang, Vec::new(), false),
InvocationKind::Derive { ref path, .. } =>
(path, MacroKind::Derive, Vec::new(), TogetherWith::None),
(path, MacroKind::Derive, Vec::new(), false),
};

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]`"),
_ => {}
if after_derive {
self.session.span_err(invoc.span(),
"macro attributes must be placed before `#[derive]`");
}
self.macro_defs.insert(invoc.expansion_data.mark, def_id);
let normal_module_def_id =
Expand Down
73 changes: 30 additions & 43 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,6 @@ 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,
Expand All @@ -238,7 +230,8 @@ pub enum InvocationKind {
attr: Option<ast::Attribute>,
traits: Vec<Path>,
item: Annotatable,
together_with: TogetherWith,
// We temporarily report errors for attribute macros placed after derives
after_derive: bool,
},
Derive {
path: Path,
Expand Down Expand Up @@ -1084,19 +1077,17 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
traits: Vec<Path>,
item: Annotatable,
kind: AstFragmentKind,
together_with: TogetherWith)
after_derive: bool)
-> AstFragment {
self.collect(kind, InvocationKind::Attr { attr, traits, item, together_with })
self.collect(kind, InvocationKind::Attr { attr, traits, item, after_derive })
}

fn find_attr_invoc(&self, attrs: &mut Vec<ast::Attribute>, together_with: &mut TogetherWith)
fn find_attr_invoc(&self, attrs: &mut Vec<ast::Attribute>, after_derive: &mut bool)
-> Option<ast::Attribute> {
let attr = attrs.iter()
.position(|a| {
if a.path == "derive" {
*together_with = TogetherWith::Derive
} else if a.path == "rustc_test_marker2" {
*together_with = TogetherWith::TestBench
*after_derive = true;
}
!attr::is_known(a) && !is_builtin_attr(a)
})
Expand All @@ -1109,19 +1100,15 @@ 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, TogetherWith)
-> (Option<ast::Attribute>, Vec<Path>, T, /* after_derive */ bool)
where T: HasAttrs,
{
let (mut attr, mut traits, mut together_with) = (None, Vec::new(), TogetherWith::None);
let (mut attr, mut traits, mut after_derive) = (None, Vec::new(), false);

item = item.map_attrs(|mut attrs| {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
Expand All @@ -1130,20 +1117,20 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
return attrs;
}

attr = self.find_attr_invoc(&mut attrs, &mut together_with);
attr = self.find_attr_invoc(&mut attrs, &mut after_derive);
traits = collect_derives(&mut self.cx, &mut attrs);
attrs
});

(attr, traits, item, together_with)
(attr, traits, item, after_derive)
}

/// 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, TogetherWith) {
let (mut attr, mut together_with) = (None, TogetherWith::None);
-> (Option<ast::Attribute>, T, /* after_derive */ bool) {
let (mut attr, mut after_derive) = (None, false);

item = item.map_attrs(|mut attrs| {
if let Some(legacy_attr_invoc) = self.cx.resolver.find_legacy_attr_invoc(&mut attrs,
Expand All @@ -1152,11 +1139,11 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
return attrs;
}

attr = self.find_attr_invoc(&mut attrs, &mut together_with);
attr = self.find_attr_invoc(&mut attrs, &mut after_derive);
attrs
});

(attr, item, together_with)
(attr, item, after_derive)
}

fn configure<T: HasAttrs>(&mut self, node: T) -> Option<T> {
Expand Down Expand Up @@ -1195,7 +1182,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, together_with) = self.classify_nonitem(expr);
let (attr, expr, after_derive) = self.classify_nonitem(expr);

if attr.is_some() {
// collect the invoc regardless of whether or not attributes are permitted here
Expand All @@ -1204,7 +1191,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, together_with).make_expr();
AstFragmentKind::Expr, after_derive).make_expr();
}

if let ast::ExprKind::Mac(mac) = expr.node {
Expand All @@ -1220,13 +1207,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, together_with) = self.classify_nonitem(expr);
let (attr, expr, after_derive) = 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, together_with).make_opt_expr();
AstFragmentKind::OptExpr, after_derive).make_opt_expr();
}

if let ast::ExprKind::Mac(mac) = expr.node {
Expand Down Expand Up @@ -1258,18 +1245,18 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {

// we'll expand attributes on expressions separately
if !stmt.is_expr() {
let (attr, derives, stmt_, together_with) = if stmt.is_item() {
let (attr, derives, stmt_, after_derive) = 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, together_with) = self.classify_nonitem(stmt);
(attr, vec![], stmt, together_with)
let (attr, stmt, after_derive) = self.classify_nonitem(stmt);
(attr, vec![], stmt, after_derive)
};

if attr.is_some() || !derives.is_empty() {
return self.collect_attr(attr, derives, Annotatable::Stmt(P(stmt_)),
AstFragmentKind::Stmts, together_with).make_stmts();
AstFragmentKind::Stmts, after_derive).make_stmts();
}

stmt = stmt_;
Expand Down Expand Up @@ -1311,10 +1298,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
fn fold_item(&mut self, item: P<ast::Item>) -> SmallVec<[P<ast::Item>; 1]> {
let item = configure!(self, item);

let (attr, traits, item, together_with) = self.classify_item(item);
let (attr, traits, item, after_derive) = self.classify_item(item);
if attr.is_some() || !traits.is_empty() {
return self.collect_attr(attr, traits, Annotatable::Item(item),
AstFragmentKind::Items, together_with).make_items();
AstFragmentKind::Items, after_derive).make_items();
}

match item.node {
Expand Down Expand Up @@ -1386,10 +1373,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
fn fold_trait_item(&mut self, item: ast::TraitItem) -> SmallVec<[ast::TraitItem; 1]> {
let item = configure!(self, item);

let (attr, traits, item, together_with) = self.classify_item(item);
let (attr, traits, item, after_derive) = self.classify_item(item);
if attr.is_some() || !traits.is_empty() {
return self.collect_attr(attr, traits, Annotatable::TraitItem(P(item)),
AstFragmentKind::TraitItems, together_with).make_trait_items()
AstFragmentKind::TraitItems, after_derive).make_trait_items()
}

match item.node {
Expand All @@ -1405,10 +1392,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
fn fold_impl_item(&mut self, item: ast::ImplItem) -> SmallVec<[ast::ImplItem; 1]> {
let item = configure!(self, item);

let (attr, traits, item, together_with) = self.classify_item(item);
let (attr, traits, item, after_derive) = self.classify_item(item);
if attr.is_some() || !traits.is_empty() {
return self.collect_attr(attr, traits, Annotatable::ImplItem(P(item)),
AstFragmentKind::ImplItems, together_with).make_impl_items();
AstFragmentKind::ImplItems, after_derive).make_impl_items();
}

match item.node {
Expand Down Expand Up @@ -1440,11 +1427,11 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
fn fold_foreign_item(&mut self, foreign_item: ast::ForeignItem)
-> SmallVec<[ast::ForeignItem; 1]>
{
let (attr, traits, foreign_item, together_with) = self.classify_item(foreign_item);
let (attr, traits, foreign_item, after_derive) = self.classify_item(foreign_item);

if attr.is_some() || !traits.is_empty() {
return self.collect_attr(attr, traits, Annotatable::ForeignItem(P(foreign_item)),
AstFragmentKind::ForeignItems, together_with)
AstFragmentKind::ForeignItems, after_derive)
.make_foreign_items();
}

Expand Down
4 changes: 0 additions & 4 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,10 +990,6 @@ 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",
Expand Down
8 changes: 1 addition & 7 deletions src/libsyntax_ext/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 mut item =
let item =
if let Annotatable::Item(i) = item { i }
else {
cx.parse_sess.span_diagnostic.span_fatal(item.span(),
Expand Down Expand Up @@ -192,12 +192,6 @@ 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),
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass-fulldeps/macro-crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
#[macro_use] #[no_link]
extern crate macro_crate_test;

#[derive(PartialEq, Clone, Debug)]
#[rustc_into_multi_foo]
#[derive(PartialEq, Clone, Debug)]
fn foo() -> AnotherFakeTypeThatHadBetterGoAway {}

// Check that the `#[into_multi_foo]`-generated `foo2` is configured away
Expand Down
20 changes: 1 addition & 19 deletions src/test/ui-fulldeps/attribute-order-restricted.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
// aux-build:attr_proc_macro.rs
// compile-flags:--test

#![feature(test)]

extern crate test;
extern crate attr_proc_macro;
use attr_proc_macro::*;

Expand All @@ -15,18 +11,4 @@ struct Before;
#[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) {}
fn main() {}
28 changes: 2 additions & 26 deletions src/test/ui-fulldeps/attribute-order-restricted.stderr
Original file line number Diff line number Diff line change
@@ -1,32 +1,8 @@
error: macro attributes must be placed before `#[derive]`
--> $DIR/attribute-order-restricted.rs:15:1
--> $DIR/attribute-order-restricted.rs:11: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
error: aborting due to previous error

0 comments on commit 8994c6d

Please sign in to comment.