Merge #2479

2479: Add expansion infrastructure for derive macros r=matklad a=flodiebold

I thought I'd experiment a bit with attribute macro/derive expansion, and here's what I've got so far. It has dummy implementations of the Copy / Clone derives, to show that the approach works; it doesn't add any attribute macro support, but I think that fits into the architecture.

Basically, during raw item collection, we look at the attributes and generate macro calls for them if necessary. Currently I only do this for derives, and just add the derive macro calls as separate calls next to the item. I think for derives, it's important that they don't obscure the actual item, since they can't actually change it (e.g. sending the item token tree through macro expansion unnecessarily might make completion within it more complicated).

Attribute macros would have to be recognized at that stage and replace the item (i.e., the raw item collector will just emit an attribute macro call, and not the item). I think when we implement this, we should try to recognize known inert attributes, so that we don't do macro expansion unnecessarily; anything that isn't known needs to be treated as a possible attribute macro call (since the raw item collector can't resolve the macro yet).

There's basically no name resolution for attribute macros implemented, I just hardcoded the built-in derives. In the future, the built-ins should work within the normal name resolution infrastructure; the problem there is that the builtin stubs in `std` use macros 2.0, which we don't support yet (and adding support is outside the scope of this).

One aspect that I don't really have a solution for, but I don't know how important it is, is removing the attribute itself from its input. I'm pretty sure rustc leaves out the attribute macro from the input, but to do that, we'd have to create a completely new syntax node. I guess we could do it when / after converting to a token tree.

Co-authored-by: Florian Diebold <[email protected]>

7 files changed, 118 insertions, 11 deletions

diff --git a/crates/ra_hir_def/src/attr.rs b/crates/ra_hir_def/src/attr.rs
index 7f9a6e7ca..2f8f02d82 100644
--- a/crates/ra_hir_def/src/attr.rs
+++ b/crates/ra_hir_def/src/attr.rs
@@ -61,7 +61,9 @@ impl Attrs {
                 AdtId::UnionId(it) => attrs_from_ast(it.lookup_intern(db).ast_id, db),
             },
             AttrDefId::TraitId(it) => attrs_from_ast(it.lookup_intern(db).ast_id, db),
-            AttrDefId::MacroDefId(it) => attrs_from_ast(it.ast_id, db),
+            AttrDefId::MacroDefId(it) => {
+                it.ast_id.map_or_else(Default::default, |ast_id| attrs_from_ast(ast_id, db))
+            }
             AttrDefId::ImplId(it) => attrs_from_ast(it.lookup_intern(db).ast_id, db),
             AttrDefId::ConstId(it) => attrs_from_loc(it.lookup(db), db),
             AttrDefId::StaticId(it) => attrs_from_loc(it.lookup(db), db),
diff --git a/crates/ra_hir_def/src/body.rs b/crates/ra_hir_def/src/body.rs
index ef1816836..7b385f3fd 100644
--- a/crates/ra_hir_def/src/body.rs
+++ b/crates/ra_hir_def/src/body.rs
@@ -6,7 +6,9 @@ pub mod scope;
 use std::{ops::Index, sync::Arc};
 
 use either::Either;
-use hir_expand::{hygiene::Hygiene, AstId, HirFileId, InFile, MacroDefId, MacroFileKind};
+use hir_expand::{
+    hygiene::Hygiene, AstId, HirFileId, InFile, MacroCallKind, MacroDefId, MacroFileKind,
+};
 use ra_arena::{map::ArenaMap, Arena};
 use ra_syntax::{ast, AstNode, AstPtr};
 use rustc_hash::FxHashMap;
@@ -46,7 +48,7 @@ impl Expander {
 
         if let Some(path) = macro_call.path().and_then(|path| self.parse_path(path)) {
             if let Some(def) = self.resolve_path_as_macro(db, &path) {
-                let call_id = def.as_call_id(db, ast_id);
+                let call_id = def.as_call_id(db, MacroCallKind::FnLike(ast_id));
                 let file_id = call_id.as_file(MacroFileKind::Expr);
                 if let Some(node) = db.parse_or_expand(file_id) {
                     if let Some(expr) = ast::Expr::cast(node) {
diff --git a/crates/ra_hir_def/src/docs.rs b/crates/ra_hir_def/src/docs.rs
index 3fc6d6934..61727bd26 100644
--- a/crates/ra_hir_def/src/docs.rs
+++ b/crates/ra_hir_def/src/docs.rs
@@ -60,7 +60,7 @@ impl Documentation {
                 docs_from_ast(&src.value[it.local_id])
             }
             AttrDefId::TraitId(it) => docs_from_ast(&it.source(db).value),
-            AttrDefId::MacroDefId(it) => docs_from_ast(&it.ast_id.to_node(db)),
+            AttrDefId::MacroDefId(it) => docs_from_ast(&it.ast_id?.to_node(db)),
             AttrDefId::ConstId(it) => docs_from_ast(&it.lookup(db).source(db).value),
             AttrDefId::StaticId(it) => docs_from_ast(&it.lookup(db).source(db).value),
             AttrDefId::FunctionId(it) => docs_from_ast(&it.lookup(db).source(db).value),
diff --git a/crates/ra_hir_def/src/nameres/collector.rs b/crates/ra_hir_def/src/nameres/collector.rs
index 9d948d4f4..08693cb13 100644
--- a/crates/ra_hir_def/src/nameres/collector.rs
+++ b/crates/ra_hir_def/src/nameres/collector.rs
@@ -4,9 +4,10 @@
 //! resolves imports and expands macros.
 
 use hir_expand::{
+    builtin_derive::find_builtin_derive,
     builtin_macro::find_builtin_macro,
     name::{self, AsName, Name},
-    HirFileId, MacroCallId, MacroDefId, MacroDefKind, MacroFileKind,
+    HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind, MacroFileKind,
 };
 use ra_cfg::CfgOptions;
 use ra_db::{CrateId, FileId};
@@ -58,6 +59,7 @@ pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> C
         glob_imports: FxHashMap::default(),
         unresolved_imports: Vec::new(),
         unexpanded_macros: Vec::new(),
+        unexpanded_attribute_macros: Vec::new(),
         mod_dirs: FxHashMap::default(),
         macro_stack_monitor: MacroStackMonitor::default(),
         poison_macros: FxHashSet::default(),
@@ -102,6 +104,7 @@ struct DefCollector<'a, DB> {
     glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, LocalImportId)>>,
     unresolved_imports: Vec<(LocalModuleId, LocalImportId, raw::ImportData)>,
     unexpanded_macros: Vec<(LocalModuleId, AstId<ast::MacroCall>, Path)>,
+    unexpanded_attribute_macros: Vec<(LocalModuleId, AstId<ast::ModuleItem>, Path)>,
     mod_dirs: FxHashMap<LocalModuleId, ModDir>,
 
     /// Some macro use `$tt:tt which mean we have to handle the macro perfectly
@@ -470,6 +473,8 @@ where
 
     fn resolve_macros(&mut self) -> ReachedFixedPoint {
         let mut macros = std::mem::replace(&mut self.unexpanded_macros, Vec::new());
+        let mut attribute_macros =
+            std::mem::replace(&mut self.unexpanded_attribute_macros, Vec::new());
         let mut resolved = Vec::new();
         let mut res = ReachedFixedPoint::Yes;
         macros.retain(|(module_id, ast_id, path)| {
@@ -482,7 +487,19 @@ where
             );
 
             if let Some(def) = resolved_res.resolved_def.take_macros() {
-                let call_id = def.as_call_id(self.db, *ast_id);
+                let call_id = def.as_call_id(self.db, MacroCallKind::FnLike(*ast_id));
+                resolved.push((*module_id, call_id, def));
+                res = ReachedFixedPoint::No;
+                return false;
+            }
+
+            true
+        });
+        attribute_macros.retain(|(module_id, ast_id, path)| {
+            let resolved_res = self.resolve_attribute_macro(path);
+
+            if let Some(def) = resolved_res {
+                let call_id = def.as_call_id(self.db, MacroCallKind::Attr(*ast_id));
                 resolved.push((*module_id, call_id, def));
                 res = ReachedFixedPoint::No;
                 return false;
@@ -492,6 +509,7 @@ where
         });
 
         self.unexpanded_macros = macros;
+        self.unexpanded_attribute_macros = attribute_macros;
 
         for (module_id, macro_call_id, macro_def_id) in resolved {
             self.collect_macro_expansion(module_id, macro_call_id, macro_def_id);
@@ -500,6 +518,20 @@ where
         res
     }
 
+    fn resolve_attribute_macro(&self, path: &Path) -> Option<MacroDefId> {
+        // FIXME this is currently super hacky, just enough to support the
+        // built-in derives
+        if let Some(name) = path.as_ident() {
+            // FIXME this should actually be handled with the normal name
+            // resolution; the std lib defines built-in stubs for the derives,
+            // but these are new-style `macro`s, which we don't support yet
+            if let Some(def_id) = find_builtin_derive(name) {
+                return Some(def_id);
+            }
+        }
+        None
+    }
+
     fn collect_macro_expansion(
         &mut self,
         module_id: LocalModuleId,
@@ -587,7 +619,9 @@ where
                         .def_collector
                         .unresolved_imports
                         .push((self.module_id, import_id, self.raw_items[import_id].clone())),
-                    raw::RawItemKind::Def(def) => self.define_def(&self.raw_items[def]),
+                    raw::RawItemKind::Def(def) => {
+                        self.define_def(&self.raw_items[def], &item.attrs)
+                    }
                     raw::RawItemKind::Macro(mac) => self.collect_macro(&self.raw_items[mac]),
                     raw::RawItemKind::Impl(imp) => {
                         let module = ModuleId {
@@ -682,10 +716,16 @@ where
         res
     }
 
-    fn define_def(&mut self, def: &raw::DefData) {
+    fn define_def(&mut self, def: &raw::DefData, attrs: &Attrs) {
         let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id };
         let ctx = LocationCtx::new(self.def_collector.db, module, self.file_id);
 
+        // FIXME: check attrs to see if this is an attribute macro invocation;
+        // in which case we don't add the invocation, just a single attribute
+        // macro invocation
+
+        self.collect_derives(attrs, def);
+
         let name = def.name.clone();
         let def: PerNs = match def.kind {
             raw::DefKind::Function(ast_id) => {
@@ -736,6 +776,23 @@ where
         self.def_collector.update(self.module_id, None, &[(name, resolution)])
     }
 
+    fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) {
+        for derive_subtree in attrs.by_key("derive").tt_values() {
+            // for #[derive(Copy, Clone)], `derive_subtree` is the `(Copy, Clone)` subtree
+            for tt in &derive_subtree.token_trees {
+                let ident = match &tt {
+                    tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => ident,
+                    tt::TokenTree::Leaf(tt::Leaf::Punct(_)) => continue, // , is ok
+                    _ => continue, // anything else would be an error (which we currently ignore)
+                };
+                let path = Path::from_tt_ident(ident);
+
+                let ast_id = AstId::new(self.file_id, def.kind.ast_id());
+                self.def_collector.unexpanded_attribute_macros.push((self.module_id, ast_id, path));
+            }
+        }
+    }
+
     fn collect_macro(&mut self, mac: &raw::MacroData) {
         let ast_id = AstId::new(self.file_id, mac.ast_id);
 
@@ -759,8 +816,8 @@ where
         if is_macro_rules(&mac.path) {
             if let Some(name) = &mac.name {
                 let macro_id = MacroDefId {
-                    ast_id,
-                    krate: self.def_collector.def_map.krate,
+                    ast_id: Some(ast_id),
+                    krate: Some(self.def_collector.def_map.krate),
                     kind: MacroDefKind::Declarative,
                 };
                 self.def_collector.define_macro(self.module_id, name.clone(), macro_id, mac.export);
@@ -773,7 +830,8 @@ where
         if let Some(macro_def) = mac.path.as_ident().and_then(|name| {
             self.def_collector.def_map[self.module_id].scope.get_legacy_macro(&name)
         }) {
-            let macro_call_id = macro_def.as_call_id(self.def_collector.db, ast_id);
+            let macro_call_id =
+                macro_def.as_call_id(self.def_collector.db, MacroCallKind::FnLike(ast_id));
 
             self.def_collector.collect_macro_expansion(self.module_id, macro_call_id, macro_def);
             return;
@@ -829,6 +887,7 @@ mod tests {
             glob_imports: FxHashMap::default(),
             unresolved_imports: Vec::new(),
             unexpanded_macros: Vec::new(),
+            unexpanded_attribute_macros: Vec::new(),
             mod_dirs: FxHashMap::default(),
             macro_stack_monitor: monitor,
             poison_macros: FxHashSet::default(),
diff --git a/crates/ra_hir_def/src/nameres/raw.rs b/crates/ra_hir_def/src/nameres/raw.rs
index de4e706c2..a2821e1c3 100644
--- a/crates/ra_hir_def/src/nameres/raw.rs
+++ b/crates/ra_hir_def/src/nameres/raw.rs
@@ -184,6 +184,21 @@ pub(super) enum DefKind {
     TypeAlias(FileAstId<ast::TypeAliasDef>),
 }
 
+impl DefKind {
+    pub fn ast_id(&self) -> FileAstId<ast::ModuleItem> {
+        match self {
+            DefKind::Function(it) => it.upcast(),
+            DefKind::Struct(it) => it.upcast(),
+            DefKind::Union(it) => it.upcast(),
+            DefKind::Enum(it) => it.upcast(),
+            DefKind::Const(it) => it.upcast(),
+            DefKind::Static(it) => it.upcast(),
+            DefKind::Trait(it) => it.upcast(),
+            DefKind::TypeAlias(it) => it.upcast(),
+        }
+    }
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub(super) struct Macro(RawId);
 impl_arena_id!(Macro);
diff --git a/crates/ra_hir_def/src/nameres/tests/macros.rs b/crates/ra_hir_def/src/nameres/tests/macros.rs
index 704065633..cfa4ecb1a 100644
--- a/crates/ra_hir_def/src/nameres/tests/macros.rs
+++ b/crates/ra_hir_def/src/nameres/tests/macros.rs
@@ -600,3 +600,27 @@ fn macro_dollar_crate_is_correct_in_indirect_deps() {
         ⋮bar: t v
     "###);
 }
+
+#[test]
+fn expand_derive() {
+    let map = compute_crate_def_map(
+        "
+        //- /main.rs
+        #[derive(Clone)]
+        struct Foo;
+        ",
+    );
+    assert_eq!(map.modules[map.root].impls.len(), 1);
+}
+
+#[test]
+fn expand_multiple_derive() {
+    let map = compute_crate_def_map(
+        "
+        //- /main.rs
+        #[derive(Copy, Clone)]
+        struct Foo;
+        ",
+    );
+    assert_eq!(map.modules[map.root].impls.len(), 2);
+}
diff --git a/crates/ra_hir_def/src/path.rs b/crates/ra_hir_def/src/path.rs
index 3030dcdf6..e547b2f03 100644
--- a/crates/ra_hir_def/src/path.rs
+++ b/crates/ra_hir_def/src/path.rs
@@ -199,6 +199,11 @@ impl Path {
         name_ref.as_name().into()
     }
 
+    /// Converts an `tt::Ident` into a single-identifier `Path`.
+    pub(crate) fn from_tt_ident(ident: &tt::Ident) -> Path {
+        ident.as_name().into()
+    }
+
     /// `true` is this path is a single identifier, like `foo`
     pub fn is_ident(&self) -> bool {
         self.kind == PathKind::Plain && self.segments.len() == 1