Skip to content

Commit e2a985e

Browse files
committed
Encode disabled proc-macros via boolean flag, not special Expander
1 parent ab50ec9 commit e2a985e

File tree

8 files changed

+90
-147
lines changed

8 files changed

+90
-147
lines changed

crates/hir-def/src/data.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,8 @@ impl<'a> AssocItemCollector<'a> {
653653
));
654654

655655
continue 'attrs;
656-
} else if exp.is_disabled() {
656+
}
657+
if exp.is_disabled() {
657658
continue 'attrs;
658659
}
659660
}

crates/hir-def/src/macro_expansion_tests/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
5858
name: "identity_when_valid".into(),
5959
kind: ProcMacroKind::Attr,
6060
expander: sync::Arc::new(IdentityWhenValidProcMacroExpander),
61+
disabled: false,
6162
},
6263
)];
6364
let db = TestDB::with_files_extra_proc_macros(ra_fixture, extra_proc_macros);

crates/hir-def/src/nameres/collector.rs

+51-61
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use either::Either;
1111
use hir_expand::{
1212
ast_id_map::FileAstId,
1313
attrs::{Attr, AttrId},
14-
builtin_attr_macro::find_builtin_attr,
14+
builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander},
1515
builtin_derive_macro::find_builtin_derive,
1616
builtin_fn_macro::find_builtin_macro,
1717
name::{name, AsName, Name},
@@ -98,12 +98,12 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
9898
};
9999
(
100100
name.as_name(),
101-
if it.expander.should_expand() {
101+
if it.disabled {
102+
CustomProcMacroExpander::disabled()
103+
} else {
102104
CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
103105
idx as u32,
104106
))
105-
} else {
106-
CustomProcMacroExpander::disabled()
107107
},
108108
)
109109
})
@@ -608,9 +608,6 @@ impl DefCollector<'_> {
608608
id: ItemTreeId<item_tree::Function>,
609609
fn_id: FunctionId,
610610
) {
611-
if self.def_map.block.is_some() {
612-
return;
613-
}
614611
let kind = def.kind.to_basedb_kind();
615612
let (expander, kind) =
616613
match self.proc_macros.as_ref().map(|it| it.iter().find(|(n, _)| n == &def.name)) {
@@ -1124,9 +1121,16 @@ impl DefCollector<'_> {
11241121
let mut push_resolved = |directive: &MacroDirective, call_id| {
11251122
resolved.push((directive.module_id, directive.depth, directive.container, call_id));
11261123
};
1124+
1125+
#[derive(PartialEq, Eq)]
1126+
enum Resolved {
1127+
Yes,
1128+
No,
1129+
}
1130+
11271131
let mut res = ReachedFixedPoint::Yes;
11281132
// Retain unresolved macros after this round of resolution.
1129-
macros.retain(|directive| {
1133+
let mut retain = |directive: &MacroDirective| {
11301134
let subns = match &directive.kind {
11311135
MacroDirectiveKind::FnLike { .. } => MacroSubNs::Bang,
11321136
MacroDirectiveKind::Attr { .. } | MacroDirectiveKind::Derive { .. } => {
@@ -1161,34 +1165,10 @@ impl DefCollector<'_> {
11611165
.scope
11621166
.add_macro_invoc(ast_id.ast_id, call_id);
11631167

1164-
let loc: MacroCallLoc = self.db.lookup_intern_macro_call(call_id);
1165-
1166-
if let MacroDefKind::ProcMacro(expander, _, _) = loc.def.kind {
1167-
if expander.is_dummy() {
1168-
// If there's no expander for the proc macro (e.g.
1169-
// because proc macros are disabled, or building the
1170-
// proc macro crate failed), report this and skip
1171-
// expansion like we would if it was disabled
1172-
self.def_map.diagnostics.push(
1173-
DefDiagnostic::unresolved_proc_macro(
1174-
directive.module_id,
1175-
loc.kind,
1176-
loc.def.krate,
1177-
),
1178-
);
1179-
1180-
res = ReachedFixedPoint::No;
1181-
return false;
1182-
} else if expander.is_disabled() {
1183-
res = ReachedFixedPoint::No;
1184-
return false;
1185-
}
1186-
}
1187-
11881168
push_resolved(directive, call_id);
11891169

11901170
res = ReachedFixedPoint::No;
1191-
return false;
1171+
return Resolved::Yes;
11921172
}
11931173
}
11941174
MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, call_site } => {
@@ -1227,7 +1207,7 @@ impl DefCollector<'_> {
12271207

12281208
push_resolved(directive, call_id);
12291209
res = ReachedFixedPoint::No;
1230-
return false;
1210+
return Resolved::Yes;
12311211
}
12321212
}
12331213
MacroDirectiveKind::Attr { ast_id: file_ast_id, mod_item, attr, tree } => {
@@ -1250,7 +1230,7 @@ impl DefCollector<'_> {
12501230
}
12511231
.collect(&[*mod_item], directive.container);
12521232
res = ReachedFixedPoint::No;
1253-
false
1233+
Resolved::Yes
12541234
};
12551235

12561236
if let Some(ident) = path.as_ident() {
@@ -1266,13 +1246,18 @@ impl DefCollector<'_> {
12661246

12671247
let def = match resolver_def_id(path.clone()) {
12681248
Some(def) if def.is_attribute() => def,
1269-
_ => return true,
1249+
_ => return Resolved::No,
12701250
};
1271-
if matches!(
1272-
def,
1273-
MacroDefId { kind: MacroDefKind::BuiltInAttr(expander, _),.. }
1274-
if expander.is_derive()
1275-
) {
1251+
1252+
if let MacroDefId {
1253+
kind:
1254+
MacroDefKind::BuiltInAttr(
1255+
BuiltinAttrExpander::Derive | BuiltinAttrExpander::DeriveConst,
1256+
_,
1257+
),
1258+
..
1259+
} = def
1260+
{
12761261
// Resolved to `#[derive]`, we don't actually expand this attribute like
12771262
// normal (as that would just be an identity expansion with extra output)
12781263
// Instead we treat derive attributes special and apply them separately.
@@ -1345,16 +1330,6 @@ impl DefCollector<'_> {
13451330
let call_id =
13461331
attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
13471332

1348-
// If proc attribute macro expansion is disabled, skip expanding it here
1349-
if !self.db.expand_proc_attr_macros() {
1350-
self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
1351-
directive.module_id,
1352-
self.db.lookup_intern_macro_call(call_id).kind,
1353-
def.krate,
1354-
));
1355-
return recollect_without(self);
1356-
}
1357-
13581333
// Skip #[test]/#[bench] expansion, which would merely result in more memory usage
13591334
// due to duplicating functions into macro expansions
13601335
if matches!(
@@ -1366,19 +1341,29 @@ impl DefCollector<'_> {
13661341
}
13671342

13681343
if let MacroDefKind::ProcMacro(exp, ..) = def.kind {
1369-
if exp.is_dummy() {
1370-
// If there's no expander for the proc macro (e.g.
1371-
// because proc macros are disabled, or building the
1372-
// proc macro crate failed), report this and skip
1373-
// expansion like we would if it was disabled
1344+
// If proc attribute macro expansion is disabled, skip expanding it here
1345+
if !self.db.expand_proc_attr_macros() {
13741346
self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
13751347
directive.module_id,
13761348
self.db.lookup_intern_macro_call(call_id).kind,
13771349
def.krate,
13781350
));
1351+
return recollect_without(self);
1352+
}
13791353

1354+
// If there's no expander for the proc macro (e.g.
1355+
// because proc macros are disabled, or building the
1356+
// proc macro crate failed), report this and skip
1357+
// expansion like we would if it was disabled
1358+
if exp.is_dummy() {
1359+
self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
1360+
directive.module_id,
1361+
self.db.lookup_intern_macro_call(call_id).kind,
1362+
def.krate,
1363+
));
13801364
return recollect_without(self);
1381-
} else if exp.is_disabled() {
1365+
}
1366+
if exp.is_disabled() {
13821367
return recollect_without(self);
13831368
}
13841369
}
@@ -1389,12 +1374,13 @@ impl DefCollector<'_> {
13891374

13901375
push_resolved(directive, call_id);
13911376
res = ReachedFixedPoint::No;
1392-
return false;
1377+
return Resolved::Yes;
13931378
}
13941379
}
13951380

1396-
true
1397-
});
1381+
Resolved::No
1382+
};
1383+
macros.retain(|it| retain(it) == Resolved::No);
13981384
// Attribute resolution can add unresolved macro invocations, so concatenate the lists.
13991385
macros.extend(mem::take(&mut self.unresolved_macros));
14001386
self.unresolved_macros = macros;
@@ -1704,7 +1690,11 @@ impl ModCollector<'_, '_> {
17041690
FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db);
17051691

17061692
let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
1707-
if self.def_collector.is_proc_macro && self.module_id == DefMap::ROOT {
1693+
1694+
if self.def_collector.def_map.block.is_none()
1695+
&& self.def_collector.is_proc_macro
1696+
&& self.module_id == DefMap::ROOT
1697+
{
17081698
if let Some(proc_macro) = attrs.parse_proc_macro_decl(&it.name) {
17091699
self.def_collector.export_proc_macro(
17101700
proc_macro,

crates/hir-def/src/nameres/diagnostics.rs

+3
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,9 @@ impl DefDiagnostic {
103103
}
104104

105105
// FIXME: Whats the difference between this and unresolved_macro_call
106+
// FIXME: This is used for a lot of things, unresolved proc macros, disabled proc macros, etc
107+
// yet the diagnostic handler in ide-diagnostics has to figure out what happened because this
108+
// struct loses all that information!
106109
pub(crate) fn unresolved_proc_macro(
107110
container: LocalModuleId,
108111
ast: MacroCallKind,

crates/hir-expand/src/proc_macro.rs

+20-25
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,6 @@ pub trait ProcMacroExpander: fmt::Debug + Send + Sync + RefUnwindSafe {
3131
call_site: Span,
3232
mixed_site: Span,
3333
) -> Result<tt::Subtree, ProcMacroExpansionError>;
34-
35-
/// If this returns `false`, expansions via [`expand`](ProcMacroExpander::expand) will always
36-
/// return the input subtree or will always return an error.
37-
///
38-
/// This is used to skip any additional expansion-related work,
39-
/// e.g. to make sure we do not touch the syntax tree in any way
40-
/// if a proc macro will never be expanded.
41-
fn should_expand(&self) -> bool {
42-
true
43-
}
4434
}
4535

4636
#[derive(Debug)]
@@ -59,38 +49,43 @@ pub struct ProcMacro {
5949
pub name: SmolStr,
6050
pub kind: ProcMacroKind,
6151
pub expander: sync::Arc<dyn ProcMacroExpander>,
52+
pub disabled: bool,
6253
}
6354

6455
#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
6556
pub struct CustomProcMacroExpander {
6657
proc_macro_id: ProcMacroId,
6758
}
6859

69-
const DUMMY_ID: u32 = !0;
70-
const DISABLED_ID: u32 = !1;
71-
7260
impl CustomProcMacroExpander {
61+
const DUMMY_ID: u32 = !0;
62+
const DISABLED_ID: u32 = !1;
63+
7364
pub fn new(proc_macro_id: ProcMacroId) -> Self {
74-
assert_ne!(proc_macro_id.0, DUMMY_ID);
65+
assert_ne!(proc_macro_id.0, Self::DUMMY_ID);
66+
assert_ne!(proc_macro_id.0, Self::DISABLED_ID);
7567
Self { proc_macro_id }
7668
}
7769

78-
pub fn dummy() -> Self {
79-
Self { proc_macro_id: ProcMacroId(DUMMY_ID) }
70+
/// A dummy expander that always errors. This is used for proc-macros that are missing, usually
71+
/// due to them not being built yet.
72+
pub const fn dummy() -> Self {
73+
Self { proc_macro_id: ProcMacroId(Self::DUMMY_ID) }
8074
}
8175

8276
/// The macro was not yet resolved.
83-
pub fn is_dummy(&self) -> bool {
84-
self.proc_macro_id.0 == DUMMY_ID
77+
pub const fn is_dummy(&self) -> bool {
78+
self.proc_macro_id.0 == Self::DUMMY_ID
8579
}
8680

87-
pub fn disabled() -> Self {
88-
Self { proc_macro_id: ProcMacroId(DISABLED_ID) }
81+
/// A dummy expander that always errors. This expander is used for macros that have been disabled.
82+
pub const fn disabled() -> Self {
83+
Self { proc_macro_id: ProcMacroId(Self::DISABLED_ID) }
8984
}
9085

9186
/// The macro is explicitly disabled and cannot be expanded.
92-
pub fn is_disabled(&self) -> bool {
93-
self.proc_macro_id.0 == DISABLED_ID
87+
pub const fn is_disabled(&self) -> bool {
88+
self.proc_macro_id.0 == Self::DISABLED_ID
9489
}
9590

9691
pub fn expand(
@@ -105,11 +100,11 @@ impl CustomProcMacroExpander {
105100
mixed_site: Span,
106101
) -> ExpandResult<tt::Subtree> {
107102
match self.proc_macro_id {
108-
ProcMacroId(DUMMY_ID) => ExpandResult::new(
103+
ProcMacroId(Self::DUMMY_ID) => ExpandResult::new(
109104
tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
110105
ExpandError::UnresolvedProcMacro(def_crate),
111106
),
112-
ProcMacroId(DISABLED_ID) => ExpandResult::new(
107+
ProcMacroId(Self::DISABLED_ID) => ExpandResult::new(
113108
tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
114109
ExpandError::MacroDisabled,
115110
),
@@ -135,7 +130,7 @@ impl CustomProcMacroExpander {
135130
);
136131
return ExpandResult::new(
137132
tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
138-
ExpandError::other("Internal error"),
133+
ExpandError::other("Internal error: proc-macro index out of bounds"),
139134
);
140135
}
141136
};

0 commit comments

Comments
 (0)