Skip to content

Commit b238ddd

Browse files
jonas-schievinkjonas-schievink
committed
Make macro def krate mandatory
Refactors builtin derive support to go through proper name resolution
1 parent c31c324 commit b238ddd

File tree

14 files changed

+91
-42
lines changed

14 files changed

+91
-42
lines changed

‎crates/hir/src/code_model.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ impl MacroDef {
970970
/// defines this macro. The reasons for this is that macros are expanded
971971
/// early, in `hir_expand`, where modules simply do not exist yet.
972972
pub fn module(self, db: &dyn HirDatabase) -> Option<Module> {
973-
let krate = self.id.krate?;
973+
let krate = self.id.krate;
974974
let module_id = db.crate_def_map(krate).root;
975975
Some(Module::new(Crate { id: krate }, module_id))
976976
}

‎crates/hir/src/semantics/source_to_def.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ impl SourceToDefCtx<'_, '_> {
158158
let krate = self.file_to_def(file_id)?.krate;
159159
let file_ast_id = self.db.ast_id_map(src.file_id).ast_id(&src.value);
160160
let ast_id = Some(AstId::new(src.file_id, file_ast_id.upcast()));
161-
Some(MacroDefId { krate: Some(krate), ast_id, kind, local_inner: false })
161+
Some(MacroDefId { krate, ast_id, kind, local_inner: false })
162162
}
163163

164164
pub(super) fn find_container(&mut self, src: InFile<&SyntaxNode>) -> Option<ChildContainer> {

‎crates/hir_def/src/body/lower.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ impl ExprCollector<'_> {
803803
}
804804
Either::Right(e) => {
805805
let mac = MacroDefId {
806-
krate: Some(self.expander.module.krate),
806+
krate: self.expander.module.krate,
807807
ast_id: Some(self.expander.ast_id(&e)),
808808
kind: MacroDefKind::Declarative,
809809
local_inner: false,

‎crates/hir_def/src/item_scope.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ impl ItemInNs {
363363
ModuleDefId::TypeAliasId(id) => id.lookup(db).module(db).krate,
364364
ModuleDefId::BuiltinType(_) => return None,
365365
},
366-
ItemInNs::Macros(id) => return id.krate,
366+
ItemInNs::Macros(id) => return Some(id.krate),
367367
})
368368
}
369369
}

‎crates/hir_def/src/nameres/collector.rs‎

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,13 @@ impl DefCollector<'_> {
309309
let macro_def = match self.proc_macros.iter().find(|(n, _)| n == name) {
310310
Some((_, expander)) => MacroDefId {
311311
ast_id: None,
312-
krate: Some(self.def_map.krate),
312+
krate: self.def_map.krate,
313313
kind: MacroDefKind::ProcMacro(*expander),
314314
local_inner: false,
315315
},
316316
None => MacroDefId {
317317
ast_id: None,
318-
krate: Some(self.def_map.krate),
318+
krate: self.def_map.krate,
319319
kind: MacroDefKind::ProcMacro(ProcMacroExpander::dummy(self.def_map.krate)),
320320
local_inner: false,
321321
},
@@ -784,14 +784,6 @@ impl DefCollector<'_> {
784784
directive: &DeriveDirective,
785785
path: &ModPath,
786786
) -> Option<MacroDefId> {
787-
if let Some(name) = path.as_ident() {
788-
// FIXME this should actually be handled with the normal name
789-
// resolution; the std lib defines built-in stubs for the derives,
790-
// but these are new-style `macro`s, which we don't support yet
791-
if let Some(def_id) = find_builtin_derive(name) {
792-
return Some(def_id);
793-
}
794-
}
795787
let resolved_res = self.def_map.resolve_path_fp_with_macro(
796788
self.db,
797789
ResolveMode::Other,
@@ -984,7 +976,9 @@ impl ModCollector<'_, '_> {
984976
// to define builtin macros, so we support at least that part.
985977
if mac.is_builtin {
986978
let krate = self.def_collector.def_map.krate;
987-
if let Some(macro_id) = find_builtin_macro(&mac.name, krate, ast_id) {
979+
let macro_id = find_builtin_macro(&mac.name, krate, ast_id)
980+
.or_else(|| find_builtin_derive(&mac.name, krate, ast_id));
981+
if let Some(macro_id) = macro_id {
988982
let vis = self
989983
.def_collector
990984
.def_map
@@ -1326,7 +1320,7 @@ impl ModCollector<'_, '_> {
13261320
// Case 2: normal `macro_rules!` macro
13271321
let macro_id = MacroDefId {
13281322
ast_id: Some(ast_id),
1329-
krate: Some(self.def_collector.def_map.krate),
1323+
krate: self.def_collector.def_map.krate,
13301324
kind: MacroDefKind::Declarative,
13311325
local_inner: mac.is_local_inner,
13321326
};

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -633,14 +633,43 @@ pub struct bar;
633633
fn expand_derive() {
634634
let map = compute_crate_def_map(
635635
"
636-
//- /main.rs
636+
//- /main.rs crate:main deps:core
637+
use core::*;
638+
637639
#[derive(Copy, Clone)]
638640
struct Foo;
641+
642+
//- /core.rs crate:core
643+
#[rustc_builtin_macro]
644+
pub macro Copy {}
645+
646+
#[rustc_builtin_macro]
647+
pub macro Clone {}
639648
",
640649
);
641650
assert_eq!(map.modules[map.root].scope.impls().len(), 2);
642651
}
643652

653+
#[test]
654+
fn resolve_builtin_derive() {
655+
check(
656+
r#"
657+
//- /main.rs crate:main deps:core
658+
use core::*;
659+
660+
//- /core.rs crate:core
661+
#[rustc_builtin_macro]
662+
pub macro Clone {}
663+
664+
pub trait Clone {}
665+
"#,
666+
expect![[r#"
667+
crate
668+
Clone: t m
669+
"#]],
670+
);
671+
}
672+
644673
#[test]
645674
fn macro_expansion_overflow() {
646675
mark::check!(macro_expansion_overflow);

‎crates/hir_expand/src/builtin_derive.rs‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use syntax::{
88
match_ast,
99
};
1010

11-
use crate::{db::AstDatabase, name, quote, LazyMacroId, MacroDefId, MacroDefKind};
11+
use crate::{db::AstDatabase, name, quote, AstId, CrateId, LazyMacroId, MacroDefId, MacroDefKind};
1212

1313
macro_rules! register_builtin {
1414
( $($trait:ident => $expand:ident),* ) => {
@@ -29,16 +29,15 @@ macro_rules! register_builtin {
2929
};
3030
expander(db, id, tt)
3131
}
32-
}
33-
34-
pub fn find_builtin_derive(ident: &name::Name) -> Option<MacroDefId> {
35-
let kind = match ident {
36-
$( id if id == &name::name![$trait] => BuiltinDeriveExpander::$trait, )*
37-
_ => return None,
38-
};
3932

40-
Some(MacroDefId { krate: None, ast_id: None, kind: MacroDefKind::BuiltInDerive(kind), local_inner: false })
33+
fn find_by_name(name: &name::Name) -> Option<Self> {
34+
match name {
35+
$( id if id == &name::name![$trait] => Some(BuiltinDeriveExpander::$trait), )*
36+
_ => None,
37+
}
38+
}
4139
}
40+
4241
};
4342
}
4443

@@ -54,6 +53,20 @@ register_builtin! {
5453
PartialEq => partial_eq_expand
5554
}
5655

56+
pub fn find_builtin_derive(
57+
ident: &name::Name,
58+
krate: CrateId,
59+
ast_id: AstId<ast::Macro>,
60+
) -> Option<MacroDefId> {
61+
let expander = BuiltinDeriveExpander::find_by_name(ident)?;
62+
Some(MacroDefId {
63+
krate,
64+
ast_id: Some(ast_id),
65+
kind: MacroDefKind::BuiltInDerive(expander),
66+
local_inner: false,
67+
})
68+
}
69+
5770
struct BasicAdtInfo {
5871
name: tt::Ident,
5972
type_params: usize,
@@ -261,7 +274,7 @@ mod tests {
261274
use super::*;
262275

263276
fn expand_builtin_derive(s: &str, name: Name) -> String {
264-
let def = find_builtin_derive(&name).unwrap();
277+
let expander = BuiltinDeriveExpander::find_by_name(&name).unwrap();
265278
let fixture = format!(
266279
r#"//- /main.rs crate:main deps:core
267280
<|>
@@ -283,7 +296,12 @@ mod tests {
283296
let attr_id = AstId::new(file_id.into(), ast_id_map.ast_id(&items[0]));
284297

285298
let loc = MacroCallLoc {
286-
def,
299+
def: MacroDefId {
300+
krate: CrateId(0),
301+
ast_id: None,
302+
kind: MacroDefKind::BuiltInDerive(expander),
303+
local_inner: false,
304+
},
287305
krate: CrateId(0),
288306
kind: MacroCallKind::Attr(attr_id, name.to_string()),
289307
};

‎crates/hir_expand/src/builtin_macro.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,13 @@ pub fn find_builtin_macro(
6969

7070
match kind {
7171
Either::Left(kind) => Some(MacroDefId {
72-
krate: Some(krate),
72+
krate,
7373
ast_id: Some(ast_id),
7474
kind: MacroDefKind::BuiltIn(kind),
7575
local_inner: false,
7676
}),
7777
Either::Right(kind) => Some(MacroDefId {
78-
krate: Some(krate),
78+
krate,
7979
ast_id: Some(ast_id),
8080
kind: MacroDefKind::BuiltInEager(kind),
8181
local_inner: false,
@@ -534,7 +534,7 @@ mod tests {
534534
Either::Left(expander) => {
535535
// the first one should be a macro_rules
536536
let def = MacroDefId {
537-
krate: Some(CrateId(0)),
537+
krate: CrateId(0),
538538
ast_id: Some(AstId::new(file_id.into(), ast_id_map.ast_id(&macro_rules))),
539539
kind: MacroDefKind::BuiltIn(expander),
540540
local_inner: false,
@@ -555,7 +555,7 @@ mod tests {
555555
Either::Right(expander) => {
556556
// the first one should be a macro_rules
557557
let def = MacroDefId {
558-
krate: Some(krate),
558+
krate,
559559
ast_id: Some(AstId::new(file_id.into(), ast_id_map.ast_id(&macro_rules))),
560560
kind: MacroDefKind::BuiltInEager(expander),
561561
local_inner: false,

‎crates/hir_expand/src/hygiene.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ impl Hygiene {
2929
MacroCallId::LazyMacro(id) => {
3030
let loc = db.lookup_intern_macro(id);
3131
match loc.def.kind {
32-
MacroDefKind::Declarative => (loc.def.krate, loc.def.local_inner),
33-
MacroDefKind::BuiltIn(_) => (loc.def.krate, false),
32+
MacroDefKind::Declarative => (Some(loc.def.krate), loc.def.local_inner),
33+
MacroDefKind::BuiltIn(_) => (Some(loc.def.krate), false),
3434
MacroDefKind::BuiltInDerive(_) => (None, false),
3535
MacroDefKind::BuiltInEager(_) => (None, false),
3636
MacroDefKind::ProcMacro(_) => (None, false),

‎crates/hir_expand/src/lib.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -224,13 +224,7 @@ impl From<EagerMacroId> for MacroCallId {
224224

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

0 commit comments

Comments
 (0)