Make macro def krate mandatory

Refactors builtin derive support to go through proper name resolution

crates/hir/src/code_model.rs

@@ -970,7 +970,7 @@ impl MacroDef {
     /// defines this macro. The reasons for this is that macros are expanded
     /// early, in `hir_expand`, where modules simply do not exist yet.
     pub fn module(self, db: &dyn HirDatabase) -> Option<Module> {
-        let krate = self.id.krate?;
+        let krate = self.id.krate;
         let module_id = db.crate_def_map(krate).root;
         Some(Module::new(Crate { id: krate }, module_id))
     }

crates/hir/src/semantics/source_to_def.rs

@@ -158,7 +158,7 @@ impl SourceToDefCtx<'_, '_> {
         let krate = self.file_to_def(file_id)?.krate;
         let file_ast_id = self.db.ast_id_map(src.file_id).ast_id(&src.value);
         let ast_id = Some(AstId::new(src.file_id, file_ast_id.upcast()));
-        Some(MacroDefId { krate: Some(krate), ast_id, kind, local_inner: false })
+        Some(MacroDefId { krate, ast_id, kind, local_inner: false })
     }
 
     pub(super) fn find_container(&mut self, src: InFile<&SyntaxNode>) -> Option<ChildContainer> {

crates/hir_def/src/body/lower.rs

@@ -803,7 +803,7 @@ impl ExprCollector<'_> {
                 }
                 Either::Right(e) => {
                     let mac = MacroDefId {
-                        krate: Some(self.expander.module.krate),
+                        krate: self.expander.module.krate,
                         ast_id: Some(self.expander.ast_id(&e)),
                         kind: MacroDefKind::Declarative,
                         local_inner: false,

crates/hir_def/src/item_scope.rs

@@ -363,7 +363,7 @@ impl ItemInNs {
                 ModuleDefId::TypeAliasId(id) => id.lookup(db).module(db).krate,
                 ModuleDefId::BuiltinType(_) => return None,
             },
-            ItemInNs::Macros(id) => return id.krate,
+            ItemInNs::Macros(id) => return Some(id.krate),
         })
     }
 }

crates/hir_def/src/nameres/collector.rs

@@ -309,13 +309,13 @@ impl DefCollector<'_> {
         let macro_def = match self.proc_macros.iter().find(|(n, _)| n == name) {
             Some((_, expander)) => MacroDefId {
                 ast_id: None,
-                krate: Some(self.def_map.krate),
+                krate: self.def_map.krate,
                 kind: MacroDefKind::ProcMacro(*expander),
                 local_inner: false,
             },
             None => MacroDefId {
                 ast_id: None,
-                krate: Some(self.def_map.krate),
+                krate: self.def_map.krate,
                 kind: MacroDefKind::ProcMacro(ProcMacroExpander::dummy(self.def_map.krate)),
                 local_inner: false,
             },
@@ -784,14 +784,6 @@ impl DefCollector<'_> {
         directive: &DeriveDirective,
         path: &ModPath,
     ) -> Option<MacroDefId> {
-        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);
-            }
-        }
         let resolved_res = self.def_map.resolve_path_fp_with_macro(
             self.db,
             ResolveMode::Other,
@@ -984,7 +976,9 @@ impl ModCollector<'_, '_> {
                     // to define builtin macros, so we support at least that part.
                     if mac.is_builtin {
                         let krate = self.def_collector.def_map.krate;
-                        if let Some(macro_id) = find_builtin_macro(&mac.name, krate, ast_id) {
+                        let macro_id = find_builtin_macro(&mac.name, krate, ast_id)
+                            .or_else(|| find_builtin_derive(&mac.name, krate, ast_id));
+                        if let Some(macro_id) = macro_id {
                             let vis = self
                                 .def_collector
                                 .def_map
@@ -1326,7 +1320,7 @@ impl ModCollector<'_, '_> {
         // Case 2: normal `macro_rules!` macro
         let macro_id = MacroDefId {
             ast_id: Some(ast_id),
-            krate: Some(self.def_collector.def_map.krate),
+            krate: self.def_collector.def_map.krate,
             kind: MacroDefKind::Declarative,
             local_inner: mac.is_local_inner,
         };

crates/hir_def/src/nameres/tests/macros.rs

@@ -633,14 +633,43 @@ pub struct bar;
 fn expand_derive() {
     let map = compute_crate_def_map(
         "
-        //- /main.rs
+        //- /main.rs crate:main deps:core
+        use core::*;
+
         #[derive(Copy, Clone)]
         struct Foo;
+
+        //- /core.rs crate:core
+        #[rustc_builtin_macro]
+        pub macro Copy {}
+
+        #[rustc_builtin_macro]
+        pub macro Clone {}
         ",
     );
     assert_eq!(map.modules[map.root].scope.impls().len(), 2);
 }
 
+#[test]
+fn resolve_builtin_derive() {
+    check(
+        r#"
+//- /main.rs crate:main deps:core
+use core::*;
+
+//- /core.rs crate:core
+#[rustc_builtin_macro]
+pub macro Clone {}
+
+pub trait Clone {}
+"#,
+        expect![[r#"
+            crate
+            Clone: t m
+        "#]],
+    );
+}
+
 #[test]
 fn macro_expansion_overflow() {
     mark::check!(macro_expansion_overflow);

crates/hir_expand/src/builtin_derive.rs

@@ -8,7 +8,7 @@ use syntax::{
     match_ast,
 };
 
-use crate::{db::AstDatabase, name, quote, LazyMacroId, MacroDefId, MacroDefKind};
+use crate::{db::AstDatabase, name, quote, AstId, CrateId, LazyMacroId, MacroDefId, MacroDefKind};
 
 macro_rules! register_builtin {
     ( $($trait:ident => $expand:ident),* ) => {
@@ -29,16 +29,15 @@ macro_rules! register_builtin {
                 };
                 expander(db, id, tt)
             }
+
+            fn find_by_name(name: &name::Name) -> Option<Self> {
+                match name {
+                    $( id if id == &name::name![$trait] => Some(BuiltinDeriveExpander::$trait), )*
+                     _ => None,
+                }
+            }
         }
 
-        pub fn find_builtin_derive(ident: &name::Name) -> Option<MacroDefId> {
-            let kind = match ident {
-                $( id if id == &name::name![$trait] => BuiltinDeriveExpander::$trait, )*
-                 _ => return None,
-            };
-
-            Some(MacroDefId { krate: None, ast_id: None, kind: MacroDefKind::BuiltInDerive(kind), local_inner: false })
-        }
     };
 }
 
@@ -54,6 +53,20 @@ register_builtin! {
     PartialEq => partial_eq_expand
 }
 
+pub fn find_builtin_derive(
+    ident: &name::Name,
+    krate: CrateId,
+    ast_id: AstId<ast::Macro>,
+) -> Option<MacroDefId> {
+    let expander = BuiltinDeriveExpander::find_by_name(ident)?;
+    Some(MacroDefId {
+        krate,
+        ast_id: Some(ast_id),
+        kind: MacroDefKind::BuiltInDerive(expander),
+        local_inner: false,
+    })
+}
+
 struct BasicAdtInfo {
     name: tt::Ident,
     type_params: usize,
@@ -261,7 +274,7 @@ mod tests {
     use super::*;
 
     fn expand_builtin_derive(s: &str, name: Name) -> String {
-        let def = find_builtin_derive(&name).unwrap();
+        let expander = BuiltinDeriveExpander::find_by_name(&name).unwrap();
         let fixture = format!(
             r#"//- /main.rs crate:main deps:core
 <|>
@@ -283,7 +296,12 @@ mod tests {
         let attr_id = AstId::new(file_id.into(), ast_id_map.ast_id(&items[0]));
 
         let loc = MacroCallLoc {
-            def,
+            def: MacroDefId {
+                krate: CrateId(0),
+                ast_id: None,
+                kind: MacroDefKind::BuiltInDerive(expander),
+                local_inner: false,
+            },
             krate: CrateId(0),
             kind: MacroCallKind::Attr(attr_id, name.to_string()),
         };

crates/hir_expand/src/builtin_macro.rs

@@ -69,13 +69,13 @@ pub fn find_builtin_macro(
 
     match kind {
         Either::Left(kind) => Some(MacroDefId {
-            krate: Some(krate),
+            krate,
             ast_id: Some(ast_id),
             kind: MacroDefKind::BuiltIn(kind),
             local_inner: false,
         }),
         Either::Right(kind) => Some(MacroDefId {
-            krate: Some(krate),
+            krate,
             ast_id: Some(ast_id),
             kind: MacroDefKind::BuiltInEager(kind),
             local_inner: false,
@@ -534,7 +534,7 @@ mod tests {
             Either::Left(expander) => {
                 // the first one should be a macro_rules
                 let def = MacroDefId {
-                    krate: Some(CrateId(0)),
+                    krate: CrateId(0),
                     ast_id: Some(AstId::new(file_id.into(), ast_id_map.ast_id(&macro_rules))),
                     kind: MacroDefKind::BuiltIn(expander),
                     local_inner: false,
@@ -555,7 +555,7 @@ mod tests {
             Either::Right(expander) => {
                 // the first one should be a macro_rules
                 let def = MacroDefId {
-                    krate: Some(krate),
+                    krate,
                     ast_id: Some(AstId::new(file_id.into(), ast_id_map.ast_id(&macro_rules))),
                     kind: MacroDefKind::BuiltInEager(expander),
                     local_inner: false,

crates/hir_expand/src/hygiene.rs

@@ -29,8 +29,8 @@ impl Hygiene {
                 MacroCallId::LazyMacro(id) => {
                     let loc = db.lookup_intern_macro(id);
                     match loc.def.kind {
-                        MacroDefKind::Declarative => (loc.def.krate, loc.def.local_inner),
-                        MacroDefKind::BuiltIn(_) => (loc.def.krate, false),
+                        MacroDefKind::Declarative => (Some(loc.def.krate), loc.def.local_inner),
+                        MacroDefKind::BuiltIn(_) => (Some(loc.def.krate), false),
                         MacroDefKind::BuiltInDerive(_) => (None, false),
                         MacroDefKind::BuiltInEager(_) => (None, false),
                         MacroDefKind::ProcMacro(_) => (None, false),

crates/hir_expand/src/lib.rs

@@ -224,13 +224,7 @@ impl From<EagerMacroId> for MacroCallId {
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct MacroDefId {
-    // FIXME: krate and ast_id are currently optional because we don't have a
-    // definition location for built-in derives. There is one, though: the
-    // standard library defines them. The problem is that it uses the new
-    // `macro` syntax for this, which we don't support yet. As soon as we do
-    // (which will probably require touching this code), we can instead use
-    // that (and also remove the hacks for resolving built-in derives).
-    pub krate: Option<CrateId>,
+    pub krate: CrateId,
     pub ast_id: Option<AstId<ast::Macro>>,
     pub kind: MacroDefKind,
 

crates/hir_ty/src/tests/macros.rs

@@ -686,6 +686,8 @@ mod clone {
     trait Clone {
         fn clone(&self) -> Self;
     }
+    #[rustc_builtin_macro]
+    macro Clone {}
 }
 "#,
     );
@@ -702,6 +704,8 @@ mod clone {
     trait Clone {
         fn clone(&self) -> Self;
     }
+    #[rustc_builtin_macro]
+    macro Clone {}
 }
 #[derive(Clone)]
 pub struct S;
@@ -737,6 +741,8 @@ mod clone {
     trait Clone {
         fn clone(&self) -> Self;
     }
+    #[rustc_builtin_macro]
+    macro Clone {}
 }
 "#,
     );

crates/ide/src/goto_implementation.rs

@@ -221,6 +221,8 @@ struct Foo<|>;
 mod marker {
     trait Copy {}
 }
+#[rustc_builtin_macro]
+macro Copy {}
 "#,
         );
     }

crates/ide/src/syntax_highlighting/test_data/highlighting.html

@@ -38,6 +38,9 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
 <pre><code><span class="keyword">use</span> <span class="module">inner</span><span class="operator">::</span><span class="punctuation">{</span><span class="self_keyword">self</span> <span class="keyword">as</span> <span class="module declaration">inner_mod</span><span class="punctuation">}</span><span class="punctuation">;</span>
 <span class="keyword">mod</span> <span class="module declaration">inner</span> <span class="punctuation">{</span><span class="punctuation">}</span>
 
+<span class="attribute attribute">#</span><span class="attribute attribute">[</span><span class="function attribute">rustc_builtin_macro</span><span class="attribute attribute">]</span>
+<span class="keyword">macro</span> <span class="unresolved_reference declaration">Copy</span> <span class="punctuation">{</span><span class="punctuation">}</span>
+
 <span class="comment">// Needed for function consuming vs normal</span>
 <span class="keyword">pub</span> <span class="keyword">mod</span> <span class="module declaration">marker</span> <span class="punctuation">{</span>
     <span class="attribute attribute">#</span><span class="attribute attribute">[</span><span class="function attribute">lang</span><span class="attribute attribute"> </span><span class="operator attribute">=</span><span class="attribute attribute"> </span><span class="string_literal attribute">"copy"</span><span class="attribute attribute">]</span>
@@ -119,7 +122,7 @@ pre                 { color: #DCDCCC; background: #3F3F3F; font-size: 22px; padd
     <span class="value_param callable">f</span><span class="punctuation">(</span><span class="punctuation">)</span>
 <span class="punctuation">}</span>
 
-<span class="keyword">fn</span> <span class="function declaration">foobar</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="operator">-&gt;</span> <span class="keyword">impl</span> <span class="unresolved_reference">Copy</span> <span class="punctuation">{</span><span class="punctuation">}</span>
+<span class="keyword">fn</span> <span class="function declaration">foobar</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="operator">-&gt;</span> <span class="keyword">impl</span> <span class="macro">Copy</span> <span class="punctuation">{</span><span class="punctuation">}</span>
 
 <span class="keyword">fn</span> <span class="function declaration">foo</span><span class="punctuation">(</span><span class="punctuation">)</span> <span class="punctuation">{</span>
     <span class="keyword">let</span> <span class="variable declaration">bar</span> <span class="operator">=</span> <span class="function">foobar</span><span class="punctuation">(</span><span class="punctuation">)</span><span class="punctuation">;</span>

crates/ide/src/syntax_highlighting/tests.rs

@@ -12,6 +12,9 @@ fn test_highlighting() {
 use inner::{self as inner_mod};
 mod inner {}
 
+#[rustc_builtin_macro]
+macro Copy {}
+
 // Needed for function consuming vs normal
 pub mod marker {
     #[lang = "copy"]