Avoid modifying invocations in place for derive helper attributes

Author: petrochenkov

src/librustc_resolve/macros.rs

@@ -20,13 +20,12 @@ use rustc::hir::map::{self, DefCollector};
 use rustc::{ty, lint};
 use rustc::middle::cstore::CrateStore;
 use syntax::ast::{self, Name, Ident};
-use syntax::attr::{self, HasAttrs};
+use syntax::attr;
 use syntax::errors::DiagnosticBuilder;
-use syntax::ext::base::{self, Annotatable, Determinacy, MultiModifier, MultiDecorator};
+use syntax::ext::base::{self, Determinacy, MultiModifier, MultiDecorator};
 use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
-use syntax::ext::expand::{self, AstFragment, AstFragmentKind, Invocation, InvocationKind};
+use syntax::ext::expand::{AstFragment, Invocation, InvocationKind};
 use syntax::ext::hygiene::{self, Mark};
-use syntax::ext::placeholders::placeholder;
 use syntax::ext::tt::macro_rules;
 use syntax::feature_gate::{self, feature_err, emit_feature_err, is_builtin_attr_name, GateIssue};
 use syntax::fold::{self, Folder};
@@ -320,7 +319,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
         None
     }
 
-    fn resolve_invoc(&mut self, invoc: &mut Invocation, scope: Mark, force: bool)
+    fn resolve_invoc(&mut self, invoc: &Invocation, scope: Mark, force: bool)
                      -> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
         let def = match invoc.kind {
             InvocationKind::Attr { attr: None, .. } => return Ok(None),
@@ -330,17 +329,22 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> {
             self.report_proc_macro_stub(invoc.span());
             return Err(Determinacy::Determined);
         } else if let Def::NonMacroAttr(attr_kind) = def {
-            let is_attr = if let InvocationKind::Attr { .. } = invoc.kind { true } else { false };
-            if is_attr && attr_kind == NonMacroAttrKind::Tool {
-                if !self.session.features_untracked().tool_attributes {
-                    feature_err(&self.session.parse_sess, "tool_attributes",
-                                invoc.span(), GateIssue::Language,
-                                "tool attributes are unstable").emit();
+            let is_attr_invoc =
+                if let InvocationKind::Attr { .. } = invoc.kind { true } else { false };
+            match attr_kind {
+                NonMacroAttrKind::Tool | NonMacroAttrKind::DeriveHelper if is_attr_invoc => {
+                    if attr_kind == NonMacroAttrKind::Tool &&
+                       !self.session.features_untracked().tool_attributes {
+                        feature_err(&self.session.parse_sess, "tool_attributes",
+                                    invoc.span(), GateIssue::Language,
+                                    "tool attributes are unstable").emit();
+                    }
+                    return Ok(Some(Lrc::new(SyntaxExtension::NonMacroAttr)));
+                }
+                _ => {
+                    self.report_non_macro_attr(invoc.path_span(), def);
+                    return Err(Determinacy::Determined);
                 }
-                return Ok(Some(Lrc::new(SyntaxExtension::NonMacroAttr)));
-            } else {
-                self.report_non_macro_attr(invoc.path_span(), def);
-                return Err(Determinacy::Determined);
             }
         }
         let def_id = def.def_id();
@@ -401,10 +405,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         self.session.span_err(span, &format!("expected a macro, found {}", def.kind_name()));
     }
 
-    fn resolve_invoc_to_def(&mut self, invoc: &mut Invocation, scope: Mark, force: bool)
+    fn resolve_invoc_to_def(&mut self, invoc: &Invocation, scope: Mark, force: bool)
                             -> Result<Def, Determinacy> {
-        let (attr, traits, item) = match invoc.kind {
-            InvocationKind::Attr { ref mut attr, ref traits, ref mut item } => (attr, traits, item),
+        let (attr, traits) = match invoc.kind {
+            InvocationKind::Attr { ref attr, ref traits, .. } => (attr, traits),
             InvocationKind::Bang { ref mac, .. } => {
                 return self.resolve_macro_to_def(scope, &mac.node.path, MacroKind::Bang, force);
             }
@@ -413,7 +417,6 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             }
         };
 
-
         let path = attr.as_ref().unwrap().path.clone();
         let mut determinacy = Determinacy::Determined;
         match self.resolve_macro_to_def(scope, &path, MacroKind::Attr, force) {
@@ -434,11 +437,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         // This loop here looks through all of the derive annotations in scope
         // and tries to resolve them. If they themselves successfully resolve
         // *and* the resolve mentions that this attribute's name is a registered
-        // custom attribute then we flag this attribute as known and update
-        // `invoc` above to point to the next invocation.
-        //
-        // By then returning `Undetermined` we should continue resolution to
-        // resolve the next attribute.
+        // custom attribute then we return that custom attribute as the resolution result.
         let attr_name = match path.segments.len() {
             1 => path.segments[0].ident.name,
             _ => return Err(determinacy),
@@ -447,20 +446,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             match self.resolve_macro(scope, path, MacroKind::Derive, force) {
                 Ok(ext) => if let SyntaxExtension::ProcMacroDerive(_, ref inert_attrs, _) = *ext {
                     if inert_attrs.contains(&attr_name) {
-                        // FIXME(jseyfried) Avoid `mem::replace` here.
-                        let dummy_item = placeholder(AstFragmentKind::Items, ast::DUMMY_NODE_ID)
-                            .make_items().pop().unwrap();
-                        let dummy_item = Annotatable::Item(dummy_item);
-                        *item = mem::replace(item, dummy_item).map_attrs(|mut attrs| {
-                            let inert_attr = attr.take().unwrap();
-                            attr::mark_known(&inert_attr);
-                            if self.use_extern_macros {
-                                *attr = expand::find_attr_invoc(&mut attrs);
-                            }
-                            attrs.push(inert_attr);
-                            attrs
-                        });
-                        return Err(Determinacy::Undetermined)
+                        return Ok(Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper));
                     }
                 },
                 Err(Determinacy::Undetermined) => determinacy = Determinacy::Undetermined,

src/libsyntax/ext/base.rs

@@ -726,7 +726,7 @@ pub trait Resolver {
     fn find_legacy_attr_invoc(&mut self, attrs: &mut Vec<Attribute>, allow_derive: bool)
                               -> Option<Attribute>;
 
-    fn resolve_invoc(&mut self, invoc: &mut Invocation, scope: Mark, force: bool)
+    fn resolve_invoc(&mut self, invoc: &Invocation, scope: Mark, force: bool)
                      -> Result<Option<Lrc<SyntaxExtension>>, Determinacy>;
     fn resolve_macro(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool)
                      -> Result<Lrc<SyntaxExtension>, Determinacy>;
@@ -754,7 +754,7 @@ impl Resolver for DummyResolver {
     fn resolve_imports(&mut self) {}
     fn find_legacy_attr_invoc(&mut self, _attrs: &mut Vec<Attribute>, _allow_derive: bool)
                               -> Option<Attribute> { None }
-    fn resolve_invoc(&mut self, _invoc: &mut Invocation, _scope: Mark, _force: bool)
+    fn resolve_invoc(&mut self, _invoc: &Invocation, _scope: Mark, _force: bool)
                      -> Result<Option<Lrc<SyntaxExtension>>, Determinacy> {
         Err(Determinacy::Determined)
     }

src/libsyntax/ext/expand.rs

@@ -252,13 +252,6 @@ impl Invocation {
             InvocationKind::Derive { ref path, .. } => path.span,
         }
     }
-
-    pub fn attr_id(&self) -> Option<ast::AttrId> {
-        match self.kind {
-            InvocationKind::Attr { attr: Some(ref attr), .. } => Some(attr.id),
-            _ => None,
-        }
-    }
 }
 
 pub struct MacroExpander<'a, 'b:'a> {
@@ -338,7 +331,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
         let mut undetermined_invocations = Vec::new();
         let (mut progress, mut force) = (false, !self.monotonic);
         loop {
-            let mut invoc = if let Some(invoc) = invocations.pop() {
+            let invoc = if let Some(invoc) = invocations.pop() {
                 invoc
             } else {
                 self.resolve_imports();
@@ -350,20 +343,10 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
 
             let scope =
                 if self.monotonic { invoc.expansion_data.mark } else { orig_expansion_data.mark };
-            let attr_id_before = invoc.attr_id();
-            let ext = match self.cx.resolver.resolve_invoc(&mut invoc, scope, force) {
+            let ext = match self.cx.resolver.resolve_invoc(&invoc, scope, force) {
                 Ok(ext) => Some(ext),
                 Err(Determinacy::Determined) => None,
                 Err(Determinacy::Undetermined) => {
-                    // Sometimes attributes which we thought were invocations
-                    // end up being custom attributes for custom derives. If
-                    // that's the case our `invoc` will have changed out from
-                    // under us. If this is the case we're making progress so we
-                    // want to flag it as such, and we test this by looking if
-                    // the `attr_id()` method has been changing over time.
-                    if invoc.attr_id() != attr_id_before {
-                        progress = true;
-                    }
                     undetermined_invocations.push(invoc);
                     continue
                 }