Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 6ae8d49

Browse files
VeykrilVeykril
committed
Simplify eager macro error handling
1 parent a5558cd commit 6ae8d49

File tree

6 files changed

+120
-211
lines changed

6 files changed

+120
-211
lines changed

‎crates/hir-def/src/body.rs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ impl Expander {
138138
db: &dyn DefDatabase,
139139
macro_call: ast::MacroCall,
140140
) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
141+
// FIXME: within_limit should support this, instead of us having to extract the error
141142
let mut unresolved_macro_err = None;
142143

143144
let result = self.within_limit(db, |this| {
@@ -146,22 +147,13 @@ impl Expander {
146147
let resolver =
147148
|path| this.resolve_path_as_macro(db, &path).map(|it| macro_id_to_def_id(db, it));
148149

149-
let mut err = None;
150-
let call_id = match macro_call.as_call_id_with_errors(
151-
db,
152-
this.def_map.krate(),
153-
resolver,
154-
&mut |e| {
155-
err.get_or_insert(e);
156-
},
157-
) {
150+
match macro_call.as_call_id_with_errors(db, this.def_map.krate(), resolver) {
158151
Ok(call_id) => call_id,
159152
Err(resolve_err) => {
160153
unresolved_macro_err = Some(resolve_err);
161-
return ExpandResult { value: None, err: None };
154+
ExpandResult { value: None, err: None }
162155
}
163-
};
164-
ExpandResult { value: call_id.ok(), err }
156+
}
165157
});
166158

167159
if let Some(err) = unresolved_macro_err {

‎crates/hir-def/src/lib.rs

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,11 @@ use hir_expand::{
6565
builtin_attr_macro::BuiltinAttrExpander,
6666
builtin_derive_macro::BuiltinDeriveExpander,
6767
builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander},
68-
eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
68+
eager::expand_eager_macro,
6969
hygiene::Hygiene,
7070
proc_macro::ProcMacroExpander,
71-
AstId, ExpandError, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId,
72-
MacroDefKind, UnresolvedMacro,
71+
AstId, ExpandError, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind,
72+
MacroDefId, MacroDefKind, UnresolvedMacro,
7373
};
7474
use item_tree::ExternBlock;
7575
use la_arena::Idx;
@@ -795,16 +795,15 @@ pub trait AsMacroCall {
795795
krate: CrateId,
796796
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
797797
) -> Option<MacroCallId> {
798-
self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()).ok()?.ok()
798+
self.as_call_id_with_errors(db, krate, resolver).ok()?.value
799799
}
800800

801801
fn as_call_id_with_errors(
802802
&self,
803803
db: &dyn db::DefDatabase,
804804
krate: CrateId,
805805
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
806-
error_sink: &mut dyn FnMut(ExpandError),
807-
) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro>;
806+
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro>;
808807
}
809808

810809
impl AsMacroCall for InFile<&ast::MacroCall> {
@@ -813,21 +812,15 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
813812
db: &dyn db::DefDatabase,
814813
krate: CrateId,
815814
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
816-
mut error_sink: &mut dyn FnMut(ExpandError),
817-
) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
815+
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
818816
let expands_to = hir_expand::ExpandTo::from_call_site(self.value);
819817
let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
820818
let h = Hygiene::new(db.upcast(), self.file_id);
821819
let path =
822820
self.value.path().and_then(|path| path::ModPath::from_src(db.upcast(), path, &h));
823821

824-
let path = match error_sink
825-
.option(path, || ExpandError::Other("malformed macro invocation".into()))
826-
{
827-
Ok(path) => path,
828-
Err(error) => {
829-
return Ok(Err(error));
830-
}
822+
let Some(path) = path else {
823+
return Ok(ExpandResult::only_err(ExpandError::Other("malformed macro invocation".into())));
831824
};
832825

833826
macro_call_as_call_id(
@@ -836,7 +829,6 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
836829
expands_to,
837830
krate,
838831
resolver,
839-
error_sink,
840832
)
841833
}
842834
}
@@ -860,21 +852,23 @@ fn macro_call_as_call_id(
860852
expand_to: ExpandTo,
861853
krate: CrateId,
862854
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
863-
error_sink: &mut dyn FnMut(ExpandError),
864-
) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
855+
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
865856
let def =
866857
resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;
867858

868859
let res = if let MacroDefKind::BuiltInEager(..) = def.kind {
869860
let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db.upcast()));
870861

871-
expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver, error_sink)?
862+
expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver)?
872863
} else {
873-
Ok(def.as_lazy_macro(
874-
db.upcast(),
875-
krate,
876-
MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },
877-
))
864+
ExpandResult {
865+
value: Some(def.as_lazy_macro(
866+
db.upcast(),
867+
krate,
868+
MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },
869+
)),
870+
err: None,
871+
}
878872
};
879873
Ok(res)
880874
}

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,21 +125,15 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
125125

126126
for macro_call in source_file.syntax().descendants().filter_map(ast::MacroCall::cast) {
127127
let macro_call = InFile::new(source.file_id, &macro_call);
128-
let mut error = None;
129-
let macro_call_id = macro_call
130-
.as_call_id_with_errors(
131-
&db,
132-
krate,
133-
|path| {
134-
resolver.resolve_path_as_macro(&db, &path).map(|it| macro_id_to_def_id(&db, it))
135-
},
136-
&mut |err| error = Some(err),
137-
)
138-
.unwrap()
128+
let res = macro_call
129+
.as_call_id_with_errors(&db, krate, |path| {
130+
resolver.resolve_path_as_macro(&db, &path).map(|it| macro_id_to_def_id(&db, it))
131+
})
139132
.unwrap();
133+
let macro_call_id = res.value.unwrap();
140134
let macro_file = MacroFile { macro_call_id };
141135
let mut expansion_result = db.parse_macro_expansion(macro_file);
142-
expansion_result.err = expansion_result.err.or(error);
136+
expansion_result.err = expansion_result.err.or(res.err);
143137
expansions.push((macro_call.value.clone(), expansion_result, db.macro_arg(macro_call_id)));
144138
}
145139

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

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use hir_expand::{
1616
builtin_fn_macro::find_builtin_macro,
1717
name::{name, AsName, Name},
1818
proc_macro::ProcMacroExpander,
19-
ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId,
20-
MacroDefKind,
19+
ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc,
20+
MacroDefId, MacroDefKind,
2121
};
2222
use itertools::{izip, Itertools};
2323
use la_arena::Idx;
@@ -1116,9 +1116,8 @@ impl DefCollector<'_> {
11161116
*expand_to,
11171117
self.def_map.krate,
11181118
resolver_def_id,
1119-
&mut |_err| (),
11201119
);
1121-
if let Ok(Ok(call_id)) = call_id {
1120+
if let Ok(ExpandResult { value: Some(call_id), .. }) = call_id {
11221121
push_resolved(directive, call_id);
11231122
res = ReachedFixedPoint::No;
11241123
return false;
@@ -1414,7 +1413,6 @@ impl DefCollector<'_> {
14141413
.take_macros()
14151414
.map(|it| macro_id_to_def_id(self.db, it))
14161415
},
1417-
&mut |_| (),
14181416
);
14191417
if let Err(UnresolvedMacro { path }) = macro_call_as_call_id {
14201418
self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
@@ -2112,7 +2110,6 @@ impl ModCollector<'_, '_> {
21122110
let ast_id = AstIdWithPath::new(self.file_id(), mac.ast_id, ModPath::clone(&mac.path));
21132111

21142112
// Case 1: try to resolve in legacy scope and expand macro_rules
2115-
let mut error = None;
21162113
match macro_call_as_call_id(
21172114
self.def_collector.db,
21182115
&ast_id,
@@ -2133,21 +2130,20 @@ impl ModCollector<'_, '_> {
21332130
)
21342131
})
21352132
},
2136-
&mut |err| {
2137-
error.get_or_insert(err);
2138-
},
21392133
) {
2140-
Ok(Ok(macro_call_id)) => {
2134+
Ok(res) => {
21412135
// Legacy macros need to be expanded immediately, so that any macros they produce
21422136
// are in scope.
2143-
self.def_collector.collect_macro_expansion(
2144-
self.module_id,
2145-
macro_call_id,
2146-
self.macro_depth + 1,
2147-
container,
2148-
);
2137+
if let Some(val) = res.value {
2138+
self.def_collector.collect_macro_expansion(
2139+
self.module_id,
2140+
val,
2141+
self.macro_depth + 1,
2142+
container,
2143+
);
2144+
}
21492145

2150-
if let Some(err) = error {
2146+
if let Some(err) = res.err {
21512147
self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error(
21522148
self.module_id,
21532149
MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to: mac.expand_to },
@@ -2157,16 +2153,6 @@ impl ModCollector<'_, '_> {
21572153

21582154
return;
21592155
}
2160-
Ok(Err(_)) => {
2161-
// Built-in macro failed eager expansion.
2162-
2163-
self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error(
2164-
self.module_id,
2165-
MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to: mac.expand_to },
2166-
error.unwrap().to_string(),
2167-
));
2168-
return;
2169-
}
21702156
Err(UnresolvedMacro { .. }) => (),
21712157
}
21722158

‎crates/hir-expand/src/db.rs

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -279,25 +279,28 @@ fn parse_macro_expansion(
279279
let mbe::ValueResult { value, err } = db.macro_expand(macro_file.macro_call_id);
280280

281281
if let Some(err) = &err {
282-
// Note:
283-
// The final goal we would like to make all parse_macro success,
284-
// such that the following log will not call anyway.
285-
let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);
286-
let node = loc.kind.to_node(db);
287-
288-
// collect parent information for warning log
289-
let parents =
290-
std::iter::successors(loc.kind.file_id().call_node(db), |it| it.file_id.call_node(db))
291-
.map(|n| format!("{:#}", n.value))
292-
.collect::<Vec<_>>()
293-
.join("\n");
294-
295-
tracing::debug!(
296-
"fail on macro_parse: (reason: {:?} macro_call: {:#}) parents: {}",
297-
err,
298-
node.value,
299-
parents
300-
);
282+
if tracing::enabled!(tracing::Level::DEBUG) {
283+
// Note:
284+
// The final goal we would like to make all parse_macro success,
285+
// such that the following log will not call anyway.
286+
let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);
287+
let node = loc.kind.to_node(db);
288+
289+
// collect parent information for warning log
290+
let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| {
291+
it.file_id.call_node(db)
292+
})
293+
.map(|n| format!("{:#}", n.value))
294+
.collect::<Vec<_>>()
295+
.join("\n");
296+
297+
tracing::debug!(
298+
"fail on macro_parse: (reason: {:?} macro_call: {:#}) parents: {}",
299+
err,
300+
node.value,
301+
parents
302+
);
303+
}
301304
}
302305
let tt = match value {
303306
Some(tt) => tt,
@@ -466,7 +469,8 @@ fn macro_expand(
466469
Ok(it) => it,
467470
// FIXME: This is weird -- we effectively report macro *definition*
468471
// errors lazily, when we try to expand the macro. Instead, they should
469-
// be reported at the definition site (when we construct a def map).
472+
// be reported at the definition site when we construct a def map.
473+
// (Note we do report them also at the definition site in the late diagnostic pass)
470474
Err(err) => {
471475
return ExpandResult::only_err(ExpandError::Other(
472476
format!("invalid macro definition: {err}").into(),

0 commit comments

Comments
 (0)