Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Temporarily prohibit proc macro attributes placed after derives
... and also proc macro attributes used together with test/bench.
  • Loading branch information
petrochenkov committed Sep 16, 2018
1 parent 32dc5a0 commit 229df02
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 51 deletions.
21 changes: 15 additions & 6 deletions src/librustc_resolve/macros.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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;
Expand Down
98 changes: 59 additions & 39 deletions src/libsyntax/ext/expand.rs
Expand Up @@ -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,
Expand All @@ -226,6 +234,7 @@ pub enum InvocationKind {
attr: Option<ast::Attribute>,
traits: Vec<Path>,
item: Annotatable,
together_with: TogetherWith,
},
Derive {
path: Path,
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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() &&
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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> {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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_;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions src/libsyntax/feature_gate.rs
Expand Up @@ -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",
Expand Down
8 changes: 7 additions & 1 deletion src/libsyntax/ptr.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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())
Expand Down
8 changes: 7 additions & 1 deletion src/libsyntax_ext/test.rs
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 item =
let mut item =
if let Annotatable::Item(i) = item { i }
else {
cx.parse_sess.span_diagnostic.span_fatal(item.span(),
Expand Down Expand Up @@ -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),
Expand Down
Expand Up @@ -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() {}
32 changes: 32 additions & 0 deletions 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) {}

0 comments on commit 229df02

Please sign in to comment.