Skip to content

Commit d1632c2

Browse files
VeykrilVeykril
committed
Report syntax errors from item level macro expansions
1 parent 71b50f9 commit d1632c2

File tree

11 files changed

+147
-76
lines changed

11 files changed

+147
-76
lines changed

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

Lines changed: 21 additions & 21 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,7 @@ 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> {
141141
// FIXME: within_limit should support this, instead of us having to extract the error
142142
let mut unresolved_macro_err = None;
143143

@@ -167,37 +167,37 @@ impl Expander {
167167
&mut self,
168168
db: &dyn DefDatabase,
169169
call_id: MacroCallId,
170-
) -> ExpandResult<Option<(Mark, T)>> {
170+
) -> ExpandResult<Option<(Mark, Parse<T>)>> {
171171
self.within_limit(db, |_this| ExpandResult::ok(Some(call_id)))
172172
}
173173

174174
fn enter_expand_inner(
175175
db: &dyn DefDatabase,
176176
call_id: MacroCallId,
177-
mut err: Option<ExpandError>,
178-
) -> ExpandResult<Option<(HirFileId, SyntaxNode)>> {
179-
if err.is_none() {
180-
err = db.macro_expand_error(call_id);
181-
}
182-
177+
mut error: Option<ExpandError>,
178+
) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
183179
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+
}
184185

185-
let raw_node = match db.parse_or_expand_with_err(file_id) {
186-
// FIXME: report parse errors
187-
Some(it) => it.syntax_node(),
186+
let parse = match value {
187+
Some(it) => it,
188188
None => {
189189
// Only `None` if the macro expansion produced no usable AST.
190-
if err.is_none() {
190+
if error.is_none() {
191191
tracing::warn!("no error despite `parse_or_expand` failing");
192192
}
193193

194-
return ExpandResult::only_err(err.unwrap_or_else(|| {
194+
return ExpandResult::only_err(error.unwrap_or_else(|| {
195195
ExpandError::Other("failed to parse macro invocation".into())
196196
}));
197197
}
198198
};
199199

200-
ExpandResult { value: Some((file_id, raw_node)), err }
200+
ExpandResult { value: Some(InFile::new(file_id, parse)), err: error }
201201
}
202202

203203
pub fn exit(&mut self, db: &dyn DefDatabase, mut mark: Mark) {
@@ -259,7 +259,7 @@ impl Expander {
259259
&mut self,
260260
db: &dyn DefDatabase,
261261
op: F,
262-
) -> ExpandResult<Option<(Mark, T)>>
262+
) -> ExpandResult<Option<(Mark, Parse<T>)>>
263263
where
264264
F: FnOnce(&mut Self) -> ExpandResult<Option<MacroCallId>>,
265265
{
@@ -286,15 +286,15 @@ impl Expander {
286286
};
287287

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

292292
self.recursion_depth += 1;
293-
self.cfg_expander.hygiene = Hygiene::new(db.upcast(), new_file_id);
294-
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);
295295
let mark =
296296
Mark { file_id: old_file_id, bomb: DropBomb::new("expansion mark dropped") };
297-
Some((mark, node))
297+
Some((mark, parse))
298298
})
299299
})
300300
}

‎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: 43 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,53 @@ 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+
// TODO: These are the wrong errors to report, report in collect_macro_items instead
642+
let file_id = self.expander.current_file_id();
643+
let root = self.db.parse_or_expand(file_id);
644+
if let Some(root) = root {
650645
let call = &item_tree[call];
651646

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);
647+
let ast_id_map = self.db.ast_id_map(file_id);
648+
let macro_call = ast_id_map.get(call.ast_id).to_node(&root);
649+
let _cx = stdx::panic_context::enter(format!(
650+
"collect_items MacroCall: {macro_call}"
651+
));
652+
if let Ok(res) =
653+
self.expander.enter_expand::<ast::MacroItems>(self.db, macro_call)
654+
{
655+
self.collect_macro_items(res, &|| hir_expand::MacroCallKind::FnLike {
656+
ast_id: InFile::new(file_id, call.ast_id),
657+
expand_to: hir_expand::ExpandTo::Items,
658+
});
660659
}
661660
}
662661
}
663662
}
664663
}
665664
}
666665

667-
fn collect_macro_items(&mut self, mark: Mark) {
666+
fn collect_macro_items(
667+
&mut self,
668+
ExpandResult { value, err }: ExpandResult<Option<(Mark, Parse<ast::MacroItems>)>>,
669+
error_call_kind: &dyn Fn() -> hir_expand::MacroCallKind,
670+
) {
671+
let Some((mark, parse)) = value else { return };
672+
673+
if let Some(err) = err {
674+
self.inactive_diagnostics.push(DefDiagnostic::macro_error(
675+
self.module_id.local_id,
676+
error_call_kind(),
677+
err.to_string(),
678+
));
679+
}
680+
if let errors @ [_, ..] = parse.errors() {
681+
self.inactive_diagnostics.push(DefDiagnostic::macro_expansion_parse_error(
682+
self.module_id.local_id,
683+
error_call_kind(),
684+
errors.into(),
685+
));
686+
}
687+
668688
let tree_id = item_tree::TreeId::new(self.expander.current_file_id(), None);
669689
let item_tree = tree_id.item_tree(self.db);
670690
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/nameres/diagnostics.rs‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ use base_db::CrateId;
44
use cfg::{CfgExpr, CfgOptions};
55
use hir_expand::{attrs::AttrId, MacroCallKind};
66
use la_arena::Idx;
7-
use syntax::ast::{self, AnyHasAttrs};
7+
use syntax::{
8+
ast::{self, AnyHasAttrs},
9+
SyntaxError,
10+
};
811

912
use crate::{
1013
item_tree::{self, ItemTreeId},
@@ -29,6 +32,8 @@ pub enum DefDiagnosticKind {
2932

3033
MacroError { ast: MacroCallKind, message: String },
3134

35+
MacroExpansionParseError { ast: MacroCallKind, errors: Box<[SyntaxError]> },
36+
3237
UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },
3338

3439
InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },
@@ -91,14 +96,28 @@ impl DefDiagnostic {
9196
Self { in_module: container, kind: DefDiagnosticKind::UnresolvedProcMacro { ast, krate } }
9297
}
9398

94-
pub(super) fn macro_error(
99+
pub(crate) fn macro_error(
95100
container: LocalModuleId,
96101
ast: MacroCallKind,
97102
message: String,
98103
) -> Self {
99104
Self { in_module: container, kind: DefDiagnosticKind::MacroError { ast, message } }
100105
}
101106

107+
pub(crate) fn macro_expansion_parse_error(
108+
container: LocalModuleId,
109+
ast: MacroCallKind,
110+
errors: &[SyntaxError],
111+
) -> Self {
112+
Self {
113+
in_module: container,
114+
kind: DefDiagnosticKind::MacroExpansionParseError {
115+
ast,
116+
errors: errors.to_vec().into_boxed_slice(),
117+
},
118+
}
119+
}
120+
102121
pub(super) fn unresolved_macro_call(
103122
container: LocalModuleId,
104123
ast: MacroCallKind,

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ pub trait ExpandDatabase: SourceDatabase {
100100
#[salsa::transparent]
101101
fn parse_or_expand(&self, file_id: HirFileId) -> Option<SyntaxNode>;
102102
#[salsa::transparent]
103-
fn parse_or_expand_with_err(&self, file_id: HirFileId) -> Option<Parse<SyntaxNode>>;
103+
fn parse_or_expand_with_err(
104+
&self,
105+
file_id: HirFileId,
106+
) -> ExpandResult<Option<Parse<SyntaxNode>>>;
104107
/// Implementation for the macro case.
105108
fn parse_macro_expansion(
106109
&self,
@@ -262,11 +265,11 @@ fn parse_or_expand(db: &dyn ExpandDatabase, file_id: HirFileId) -> Option<Syntax
262265
fn parse_or_expand_with_err(
263266
db: &dyn ExpandDatabase,
264267
file_id: HirFileId,
265-
) -> Option<Parse<SyntaxNode>> {
268+
) -> ExpandResult<Option<Parse<SyntaxNode>>> {
266269
match file_id.repr() {
267-
HirFileIdRepr::FileId(file_id) => Some(db.parse(file_id).to_syntax()),
270+
HirFileIdRepr::FileId(file_id) => ExpandResult::ok(Some(db.parse(file_id).to_syntax())),
268271
HirFileIdRepr::MacroFile(macro_file) => {
269-
db.parse_macro_expansion(macro_file).value.map(|(parse, _)| parse)
272+
db.parse_macro_expansion(macro_file).map(|it| it.map(|(parse, _)| parse))
270273
}
271274
}
272275
}

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
use std::sync::Arc;
2222

2323
use base_db::CrateId;
24-
use syntax::{ted, SyntaxNode};
24+
use syntax::{ted, Parse, SyntaxNode};
2525

2626
use crate::{
2727
ast::{self, AstNode},
@@ -111,7 +111,7 @@ fn lazy_expand(
111111
def: &MacroDefId,
112112
macro_call: InFile<ast::MacroCall>,
113113
krate: CrateId,
114-
) -> ExpandResult<Option<InFile<SyntaxNode>>> {
114+
) -> ExpandResult<Option<InFile<Parse<SyntaxNode>>>> {
115115
let ast_id = db.ast_id_map(macro_call.file_id).ast_id(&macro_call.value);
116116

117117
let expand_to = ExpandTo::from_call_site(&macro_call.value);
@@ -121,13 +121,8 @@ fn lazy_expand(
121121
MacroCallKind::FnLike { ast_id: macro_call.with_value(ast_id), expand_to },
122122
);
123123

124-
let err = db.macro_expand_error(id);
125-
let value =
126-
db.parse_or_expand_with_err(id.as_file()).map(|node| InFile::new(id.as_file(), node));
127-
// FIXME: report parse errors
128-
let value = value.map(|it| it.map(|it| it.syntax_node()));
129-
130-
ExpandResult { value, err }
124+
db.parse_or_expand_with_err(id.as_file())
125+
.map(|parse| parse.map(|parse| InFile::new(id.as_file(), parse)))
131126
}
132127

133128
fn eager_macro_recur(
@@ -183,8 +178,14 @@ fn eager_macro_recur(
183178
Some(val) => {
184179
// replace macro inside
185180
let hygiene = Hygiene::new(db, val.file_id);
186-
let ExpandResult { value, err: error } =
187-
eager_macro_recur(db, &hygiene, val, krate, macro_resolver)?;
181+
let ExpandResult { value, err: error } = eager_macro_recur(
182+
db,
183+
&hygiene,
184+
// FIXME: We discard parse errors here
185+
val.map(|it| it.syntax_node()),
186+
krate,
187+
macro_resolver,
188+
)?;
188189
let err = if err.is_none() { error } else { err };
189190
ExpandResult { value, err }
190191
}

‎crates/hir-ty/src/lower.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,8 @@ impl<'a> TyLoweringContext<'a> {
381381
match expander.enter_expand::<ast::Type>(self.db.upcast(), macro_call) {
382382
Ok(ExpandResult { value: Some((mark, expanded)), .. }) => {
383383
let ctx = expander.ctx(self.db.upcast());
384-
let type_ref = TypeRef::from_ast(&ctx, expanded);
384+
// FIXME: Report syntax errors in expansion here
385+
let type_ref = TypeRef::from_ast(&ctx, expanded.tree());
385386

386387
drop(expander);
387388
let ty = self.lower_ty(&type_ref);

‎crates/hir/src/diagnostics.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use cfg::{CfgExpr, CfgOptions};
1010
use either::Either;
1111
use hir_def::path::ModPath;
1212
use hir_expand::{name::Name, HirFileId, InFile};
13-
use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange};
13+
use syntax::{ast, AstPtr, SyntaxError, SyntaxNodePtr, TextRange};
1414

1515
use crate::{AssocItem, Field, Local, MacroKind, Type};
1616

@@ -38,8 +38,9 @@ diagnostics![
3838
IncorrectCase,
3939
InvalidDeriveTarget,
4040
IncoherentImpl,
41-
MacroError,
4241
MacroDefError,
42+
MacroError,
43+
MacroExpansionParseError,
4344
MalformedDerive,
4445
MismatchedArgCount,
4546
MissingFields,
@@ -132,6 +133,13 @@ pub struct MacroError {
132133
pub message: String,
133134
}
134135

136+
#[derive(Debug, Clone, Eq, PartialEq)]
137+
pub struct MacroExpansionParseError {
138+
pub node: InFile<SyntaxNodePtr>,
139+
pub precise_location: Option<TextRange>,
140+
pub errors: Box<[SyntaxError]>,
141+
}
142+
135143
#[derive(Debug, Clone, Eq, PartialEq)]
136144
pub struct MacroDefError {
137145
pub node: InFile<AstPtr<ast::Macro>>,

0 commit comments

Comments
 (0)