Skip to content

Commit

Permalink
Auto merge of #39572 - jseyfried:fix_inert_attributes, r=nrc
Browse files Browse the repository at this point in the history
macros: fix inert attributes from `proc_macro_derives` with `#![feature(proc_macro)]`

This PR refactors collection of `proc_macro_derive` invocations to fix #39347.

After this PR, the input to a `#[proc_macro_derive]` function no longer sees `#[derive]`s on the underlying item. For example, consider:
```rust
extern crate my_derives;
use my_derives::{Trait, Trait2};

#[derive(Copy, Clone)]
#[derive(Trait)]
#[derive(Trait2)]
struct S;
```

Today, the input to the `Trait` derive is `#[derive(Copy, Clone, Trait2)] struct S;`, and the input to the `Trait2` derive is `#[derive(Copy, Clone)] struct S;`. More generally, a `proc_macro_derive` sees all builtin derives, as well as all `proc_macro_derive`s listed *after* the one being invoked.

After this PR, both `Trait` and `Trait2` will see `struct S;`.
This is a [breaking-change], but I believe it is highly unlikely to cause breakage in practice.

r? @nrc
  • Loading branch information
bors committed Feb 12, 2017
2 parents 81bd267 + 2cc61ee commit 956e2bc
Show file tree
Hide file tree
Showing 18 changed files with 325 additions and 313 deletions.
7 changes: 3 additions & 4 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,9 @@ impl<'a> CrateLoader<'a> {
trait_name: &str,
expand: fn(TokenStream) -> TokenStream,
attributes: &[&'static str]) {
let attrs = attributes.iter().cloned().map(Symbol::intern).collect();
let derive = SyntaxExtension::ProcMacroDerive(
Box::new(ProcMacroDerive::new(expand, attrs))
);
let attrs = attributes.iter().cloned().map(Symbol::intern).collect::<Vec<_>>();
let derive = ProcMacroDerive::new(expand, attrs.clone());
let derive = SyntaxExtension::ProcMacroDerive(Box::new(derive), attrs);
self.0.push((Symbol::intern(trait_name), Rc::new(derive)));
}

Expand Down
76 changes: 62 additions & 14 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ use syntax::ext::base::{NormalTT, Resolver as SyntaxResolver, SyntaxExtension};
use syntax::ext::expand::{Expansion, mark_tts};
use syntax::ext::hygiene::Mark;
use syntax::ext::tt::macro_rules;
use syntax::feature_gate::{emit_feature_err, GateIssue, is_builtin_attr};
use syntax::feature_gate::{self, emit_feature_err, GateIssue};
use syntax::fold::{self, Folder};
use syntax::ptr::P;
use syntax::symbol::keywords;
use syntax::symbol::{Symbol, keywords};
use syntax::util::lev_distance::find_best_match_for_name;
use syntax::visit::Visitor;
use syntax_pos::{Span, DUMMY_SP};
Expand Down Expand Up @@ -130,12 +130,16 @@ impl<'a> base::Resolver for Resolver<'a> {
self.whitelisted_legacy_custom_derives.contains(&name)
}

fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion) {
fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion, derives: &[Mark]) {
let invocation = self.invocations[&mark];
self.collect_def_ids(invocation, expansion);

self.current_module = invocation.module.get();
self.current_module.unresolved_invocations.borrow_mut().remove(&mark);
self.current_module.unresolved_invocations.borrow_mut().extend(derives);
for &derive in derives {
self.invocations.insert(derive, invocation);
}
let mut visitor = BuildReducedGraphVisitor {
resolver: self,
legacy_scope: LegacyScope::Invocation(invocation),
Expand Down Expand Up @@ -172,7 +176,9 @@ impl<'a> base::Resolver for Resolver<'a> {
ImportResolver { resolver: self }.resolve_imports()
}

fn find_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>) -> Option<ast::Attribute> {
// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<ast::Attribute>)
-> Option<ast::Attribute> {
for i in 0..attrs.len() {
match self.builtin_macros.get(&attrs[i].name()).cloned() {
Some(binding) => match *binding.get_macro(self) {
Expand All @@ -183,11 +189,50 @@ impl<'a> base::Resolver for Resolver<'a> {
},
None => {}
}
}

if self.proc_macro_enabled && !is_builtin_attr(&attrs[i]) {
return Some(attrs.remove(i));
// Check for legacy derives
for i in 0..attrs.len() {
if attrs[i].name() == "derive" {
let mut traits = match attrs[i].meta_item_list() {
Some(traits) if !traits.is_empty() => traits.to_owned(),
_ => continue,
};

for j in 0..traits.len() {
let legacy_name = Symbol::intern(&match traits[j].word() {
Some(..) => format!("derive_{}", traits[j].name().unwrap()),
None => continue,
});
if !self.builtin_macros.contains_key(&legacy_name) {
continue
}
let span = traits.remove(j).span;
self.gate_legacy_custom_derive(legacy_name, span);
if traits.is_empty() {
attrs.remove(i);
} else {
attrs[i].value = ast::MetaItem {
name: attrs[i].name(),
span: attrs[i].span,
node: ast::MetaItemKind::List(traits),
};
}
return Some(ast::Attribute {
value: ast::MetaItem {
name: legacy_name,
span: span,
node: ast::MetaItemKind::Word,
},
id: attr::mk_attr_id(),
style: ast::AttrStyle::Outer,
is_sugared_doc: false,
span: span,
});
}
}
}

None
}

Expand Down Expand Up @@ -236,7 +281,7 @@ impl<'a> base::Resolver for Resolver<'a> {
Ok(binding) => Ok(binding.get_macro(self)),
Err(Determinacy::Undetermined) if !force => return Err(Determinacy::Undetermined),
_ => {
let msg = format!("macro undefined: '{}!'", name);
let msg = format!("macro undefined: `{}`", name);
let mut err = self.session.struct_span_err(span, &msg);
self.suggest_macro_name(&name.as_str(), &mut err);
err.emit();
Expand All @@ -251,13 +296,6 @@ impl<'a> base::Resolver for Resolver<'a> {
result
}

fn resolve_builtin_macro(&mut self, tname: Name) -> Result<Rc<SyntaxExtension>, Determinacy> {
match self.builtin_macros.get(&tname).cloned() {
Some(binding) => Ok(binding.get_macro(self)),
None => Err(Determinacy::Undetermined),
}
}

fn resolve_derive_macro(&mut self, scope: Mark, path: &ast::Path, force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy> {
let ast::Path { span, .. } = *path;
Expand Down Expand Up @@ -540,4 +578,14 @@ impl<'a> Resolver<'a> {
`use {}::{};`", crate_name, name))
.emit();
}

fn gate_legacy_custom_derive(&mut self, name: Symbol, span: Span) {
if !self.session.features.borrow().custom_derive {
let sess = &self.session.parse_sess;
let explain = feature_gate::EXPLAIN_CUSTOM_DERIVE;
emit_feature_err(sess, "custom_derive", span, GateIssue::Language, explain);
} else if !self.is_whitelisted_legacy_custom_derive(name) {
self.session.span_warn(span, feature_gate::EXPLAIN_DEPR_CUSTOM_DERIVE);
}
}
}
15 changes: 6 additions & 9 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ pub enum SyntaxExtension {
/// The input is the annotated item.
/// Allows generating code to implement a Trait for a given struct
/// or enum item.
ProcMacroDerive(Box<MultiItemModifier>),
ProcMacroDerive(Box<MultiItemModifier>, Vec<Symbol> /* inert attribute names */),

/// An attribute-like procedural macro that derives a builtin trait.
BuiltinDerive(BuiltinDeriveFn),
Expand All @@ -528,15 +528,15 @@ pub trait Resolver {
fn eliminate_crate_var(&mut self, item: P<ast::Item>) -> P<ast::Item>;
fn is_whitelisted_legacy_custom_derive(&self, name: Name) -> bool;

fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion);
fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion, derives: &[Mark]);
fn add_ext(&mut self, ident: ast::Ident, ext: Rc<SyntaxExtension>);
fn add_expansions_at_stmt(&mut self, id: ast::NodeId, macros: Vec<Mark>);

fn resolve_imports(&mut self);
fn find_attr_invoc(&mut self, attrs: &mut Vec<Attribute>) -> Option<Attribute>;
// Resolves attribute and derive legacy macros from `#![plugin(..)]`.
fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>) -> Option<Attribute>;
fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy>;
fn resolve_builtin_macro(&mut self, tname: Name) -> Result<Rc<SyntaxExtension>, Determinacy>;
fn resolve_derive_macro(&mut self, scope: Mark, path: &ast::Path, force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy>;
}
Expand All @@ -555,19 +555,16 @@ impl Resolver for DummyResolver {
fn eliminate_crate_var(&mut self, item: P<ast::Item>) -> P<ast::Item> { item }
fn is_whitelisted_legacy_custom_derive(&self, _name: Name) -> bool { false }

fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion) {}
fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion, _derives: &[Mark]) {}
fn add_ext(&mut self, _ident: ast::Ident, _ext: Rc<SyntaxExtension>) {}
fn add_expansions_at_stmt(&mut self, _id: ast::NodeId, _macros: Vec<Mark>) {}

fn resolve_imports(&mut self) {}
fn find_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>) -> Option<Attribute> { None }
fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>) -> Option<Attribute> { None }
fn resolve_macro(&mut self, _scope: Mark, _path: &ast::Path, _force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy> {
Err(Determinacy::Determined)
}
fn resolve_builtin_macro(&mut self, _tname: Name) -> Result<Rc<SyntaxExtension>, Determinacy> {
Err(Determinacy::Determined)
}
fn resolve_derive_macro(&mut self, _scope: Mark, _path: &ast::Path, _force: bool)
-> Result<Rc<SyntaxExtension>, Determinacy> {
Err(Determinacy::Determined)
Expand Down
Loading

0 comments on commit 956e2bc

Please sign in to comment.