Skip to content

Commit 697b335

Browse files
borsbors
committed
Auto merge of #14584 - Veykril:macro-def-err, r=Veykril
internal: Report item-level macro expansion syntax errors
2 parents f0a40c3 + 0f4ffaa commit 697b335

File tree

22 files changed

+329
-327
lines changed

22 files changed

+329
-327
lines changed

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

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use limit::Limit;
2121
use once_cell::unsync::OnceCell;
2222
use profile::Count;
2323
use rustc_hash::FxHashMap;
24-
use syntax::{ast, AstPtr, SyntaxNode, SyntaxNodePtr};
24+
use syntax::{ast, AstPtr, Parse, SyntaxNode, SyntaxNodePtr};
2525

2626
use crate::{
2727
attr::Attrs,
@@ -137,7 +137,8 @@ impl Expander {
137137
&mut self,
138138
db: &dyn DefDatabase,
139139
macro_call: ast::MacroCall,
140-
) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {
140+
) -> Result<ExpandResult<Option<(Mark, Parse<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 {
@@ -175,37 +167,37 @@ impl Expander {
175167
&mut self,
176168
db: &dyn DefDatabase,
177169
call_id: MacroCallId,
178-
) -> ExpandResult<Option<(Mark, T)>> {
170+
) -> ExpandResult<Option<(Mark, Parse<T>)>> {
179171
self.within_limit(db, |_this| ExpandResult::ok(Some(call_id)))
180172
}
181173

182174
fn enter_expand_inner(
183175
db: &dyn DefDatabase,
184176
call_id: MacroCallId,
185-
mut err: Option<ExpandError>,
186-
) -> ExpandResult<Option<(HirFileId, SyntaxNode)>> {
187-
if err.is_none() {
188-
err = db.macro_expand_error(call_id);
189-
}
190-
177+
mut error: Option<ExpandError>,
178+
) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
191179
let file_id = call_id.as_file();
180+
let ExpandResult { value, err } = db.parse_or_expand_with_err(file_id);
181+
182+
if error.is_none() {
183+
error = err;
184+
}
192185

193-
let raw_node = match db.parse_or_expand_with_err(file_id) {
194-
// FIXME: report parse errors
195-
Some(it) => it.syntax_node(),
186+
let parse = match value {
187+
Some(it) => it,
196188
None => {
197189
// Only `None` if the macro expansion produced no usable AST.
198-
if err.is_none() {
190+
if error.is_none() {
199191
tracing::warn!("no error despite `parse_or_expand` failing");
200192
}
201193

202-
return ExpandResult::only_err(err.unwrap_or_else(|| {
194+
return ExpandResult::only_err(error.unwrap_or_else(|| {
203195
ExpandError::Other("failed to parse macro invocation".into())
204196
}));
205197
}
206198
};
207199

208-
ExpandResult { value: Some((file_id, raw_node)), err }
200+
ExpandResult { value: Some(InFile::new(file_id, parse)), err: error }
209201
}
210202

211203
pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
@@ -267,7 +259,7 @@ impl Expander {
267259
&mut self,
268260
db: &dyn DefDatabase,
269261
op: F,
270-
) -> ExpandResult<Option<(Mark, T)>>
262+
) -> ExpandResult<Option<(Mark, Parse<T>)>>
271263
where
272264
F: FnOnce(&mut Self) -> ExpandResult<Option<MacroCallId>>,
273265
{
@@ -294,15 +286,15 @@ impl Expander {
294286
};
295287

296288
Self::enter_expand_inner(db, call_id, err).map(|value| {
297-
value.and_then(|(new_file_id, node)| {
298-
let node = T::cast(node)?;
289+
value.and_then(|InFile { file_id, value }| {
290+
let parse = value.cast::<T>()?;
299291

300292
self.recursion_depth += 1;
301-
self.cfg_expander.hygiene = Hygiene::new(db.upcast(), new_file_id);
302-
let old_file_id = std::mem::replace(&mut self.current_file_id, new_file_id);
293+
self.cfg_expander.hygiene = Hygiene::new(db.upcast(), file_id);
294+
let old_file_id = std::mem::replace(&mut self.current_file_id, file_id);
303295
let mark =
304296
Mark { file_id: old_file_id, bomb: DropBomb::new("expansion mark dropped") };
305-
Some((mark, node))
297+
Some((mark, parse))
306298
})
307299
})
308300
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,11 @@ impl ExprCollector<'_> {
824824
self.db.ast_id_map(self.expander.current_file_id),
825825
);
826826

827-
let id = collector(self, Some(expansion));
827+
if record_diagnostics {
828+
// FIXME: Report parse errors here
829+
}
830+
831+
let id = collector(self, Some(expansion.tree()));
828832
self.ast_id_map = prev_ast_id_map;
829833
self.expander.exit(self.db, mark);
830834
id

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

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::sync::Arc;
77
use hir_expand::{name::Name, AstId, ExpandResult, HirFileId, InFile, MacroCallId, MacroDefKind};
88
use intern::Interned;
99
use smallvec::SmallVec;
10-
use syntax::ast;
10+
use syntax::{ast, Parse};
1111

1212
use crate::{
1313
attr::Attrs,
@@ -604,13 +604,10 @@ impl<'a> AssocItemCollector<'a> {
604604
continue 'attrs;
605605
}
606606
}
607-
match self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id) {
608-
ExpandResult { value: Some((mark, _)), .. } => {
609-
self.collect_macro_items(mark);
610-
continue 'items;
611-
}
612-
ExpandResult { .. } => {}
613-
}
607+
608+
let res = self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id);
609+
self.collect_macro_items(res, &|| loc.kind.clone());
610+
continue 'items;
614611
}
615612
}
616613

@@ -641,30 +638,52 @@ impl<'a> AssocItemCollector<'a> {
641638
self.items.push((item.name.clone(), def.into()));
642639
}
643640
AssocItem::MacroCall(call) => {
644-
if let Some(root) =
645-
self.db.parse_or_expand_with_err(self.expander.current_file_id())
646-
{
647-
// FIXME: report parse errors
648-
let root = root.syntax_node();
649-
641+
let file_id = self.expander.current_file_id();
642+
let root = self.db.parse_or_expand(file_id);
643+
if let Some(root) = root {
650644
let call = &item_tree[call];
651645

652-
let ast_id_map = self.db.ast_id_map(self.expander.current_file_id());
653-
let call = ast_id_map.get(call.ast_id).to_node(&root);
654-
let _cx =
655-
stdx::panic_context::enter(format!("collect_items MacroCall: {call}"));
656-
let res = self.expander.enter_expand::<ast::MacroItems>(self.db, call);
657-
658-
if let Ok(ExpandResult { value: Some((mark, _)), .. }) = res {
659-
self.collect_macro_items(mark);
646+
let ast_id_map = self.db.ast_id_map(file_id);
647+
let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
648+
let _cx = stdx::panic_context::enter(format!(
649+
"collect_items MacroCall: {macro_call}"
650+
));
651+
if let Ok(res) =
652+
self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
653+
{
654+
self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
655+
ast_id: InFile::new(file_id, call.ast_id),
656+
expand_to: hir_expand::ExpandTo::Items,
657+
});
660658
}
661659
}
662660
}
663661
}
664662
}
665663
}
666664

667-
fn collect_macro_items(&mut self, mark: Mark) {
665+
fn collect_macro_items(
666+
&mut self,
667+
ExpandResult { value, err }: ExpandResult<Option<(Mark, Parse<ast::MacroItems>)>>,
668+
error_call_kind: &dyn Fn() -> hir_expand::MacroCallKind,
669+
) {
670+
let Some((mark, parse)) = value else { return };
671+
672+
if let Some(err) = err {
673+
self.inactive_diagnostics.push(DefDiagnostic::macro_error(
674+
self.module_id.local_id,
675+
error_call_kind(),
676+
err.to_string(),
677+
));
678+
}
679+
if let errors @ [_, ..] = parse.errors() {
680+
self.inactive_diagnostics.push(DefDiagnostic::macro_expansion_parse_error(
681+
self.module_id.local_id,
682+
error_call_kind(),
683+
errors.into(),
684+
));
685+
}
686+
668687
let tree_id = item_tree::TreeId::new(self.expander.current_file_id(), None);
669688
let item_tree = tree_id.item_tree(self.db);
670689
let iter: SmallVec<[_; 2]> =

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ impl GenericParams {
350350
match expander.enter_expand::<ast::Type>(db, macro_call) {
351351
Ok(ExpandResult { value: Some((mark, expanded)), .. }) => {
352352
let ctx = expander.ctx(db);
353-
let type_ref = TypeRef::from_ast(&ctx, expanded);
353+
let type_ref = TypeRef::from_ast(&ctx, expanded.tree());
354354
self.fill_implicit_impl_trait_args(db, expander, &type_ref);
355355
expander.exit(db, mark);
356356
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ pub struct ItemTree {
101101
top_level: SmallVec<[ModItem; 1]>,
102102
attrs: FxHashMap<AttrOwner, RawAttrs>,
103103

104+
// FIXME: Remove this indirection, an item tree is almost always non-empty?
104105
data: Option<Box<ItemTreeData>>,
105106
}
106107

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

Lines changed: 29 additions & 25 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,30 +812,23 @@ 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

833-
macro_call_as_call_id(
826+
macro_call_as_call_id_(
834827
db,
835828
&AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
836829
expands_to,
837830
krate,
838831
resolver,
839-
error_sink,
840832
)
841833
}
842834
}
@@ -860,21 +852,33 @@ 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<Option<MacroCallId>, UnresolvedMacro> {
856+
macro_call_as_call_id_(db, call, expand_to, krate, resolver).map(|res| res.value)
857+
}
858+
859+
fn macro_call_as_call_id_(
860+
db: &dyn db::DefDatabase,
861+
call: &AstIdWithPath<ast::MacroCall>,
862+
expand_to: ExpandTo,
863+
krate: CrateId,
864+
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
865+
) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {
865866
let def =
866867
resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;
867868

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

871-
expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver, error_sink)?
872+
expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver)?
872873
} else {
873-
Ok(def.as_lazy_macro(
874-
db.upcast(),
875-
krate,
876-
MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },
877-
))
874+
ExpandResult {
875+
value: Some(def.as_lazy_macro(
876+
db.upcast(),
877+
krate,
878+
MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },
879+
)),
880+
err: None,
881+
}
878882
};
879883
Ok(res)
880884
}

‎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

0 commit comments

Comments
 (0)