Skip to content

Commit ae9c553

Browse files
VeykrilVeykril
committed
Make basic use of spans for macro expansion errors
1 parent 7beac14 commit ae9c553

File tree

24 files changed

+387
-328
lines changed

24 files changed

+387
-328
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,7 @@ impl<'a> AssocItemCollector<'a> {
661661
self.diagnostics.push(DefDiagnostic::macro_error(
662662
self.module_id.local_id,
663663
ast_id,
664+
(*attr.path).clone(),
664665
err,
665666
));
666667
continue 'attrs;

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use base_db::CrateId;
66
use cfg::CfgOptions;
77
use drop_bomb::DropBomb;
88
use hir_expand::{
9-
attrs::RawAttrs, mod_path::ModPath, span_map::SpanMap, ExpandError, ExpandResult, HirFileId,
10-
InFile, MacroCallId,
9+
attrs::RawAttrs, mod_path::ModPath, span_map::SpanMap, ExpandError, ExpandErrorKind,
10+
ExpandResult, HirFileId, InFile, Lookup, MacroCallId,
1111
};
1212
use limit::Limit;
1313
use span::SyntaxContextId;
@@ -160,26 +160,30 @@ impl Expander {
160160
// so don't return overflow error here to avoid diagnostics duplication.
161161
cov_mark::hit!(overflow_but_not_me);
162162
return ExpandResult::ok(None);
163-
} else if self.recursion_limit.check(self.recursion_depth as usize + 1).is_err() {
164-
self.recursion_depth = u32::MAX;
165-
cov_mark::hit!(your_stack_belongs_to_me);
166-
return ExpandResult::only_err(ExpandError::RecursionOverflow);
167163
}
168164

169165
let ExpandResult { value, err } = op(self);
170166
let Some(call_id) = value else {
171167
return ExpandResult { value: None, err };
172168
};
169+
if self.recursion_limit.check(self.recursion_depth as usize + 1).is_err() {
170+
self.recursion_depth = u32::MAX;
171+
cov_mark::hit!(your_stack_belongs_to_me);
172+
return ExpandResult::only_err(ExpandError::new(
173+
db.macro_arg_considering_derives(call_id, &call_id.lookup(db.upcast()).kind).2,
174+
ExpandErrorKind::RecursionOverflow,
175+
));
176+
}
173177

174178
let macro_file = call_id.as_macro_file();
175179
let res = db.parse_macro_expansion(macro_file);
176180

177181
let err = err.or(res.err);
178182
ExpandResult {
179-
value: match err {
183+
value: match &err {
180184
// If proc-macro is disabled or unresolved, we want to expand to a missing expression
181185
// instead of an empty tree which might end up in an empty block.
182-
Some(ExpandError::MissingProcMacroExpander(_)) => None,
186+
Some(e) if matches!(e.kind(), ExpandErrorKind::MissingProcMacroExpander(_)) => None,
183187
_ => (|| {
184188
let parse = res.value.0.cast::<T>()?;
185189

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1434,7 +1434,10 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
14341434
});
14351435

14361436
let Some((call_site, path)) = path else {
1437-
return Ok(ExpandResult::only_err(ExpandError::other("malformed macro invocation")));
1437+
return Ok(ExpandResult::only_err(ExpandError::other(
1438+
span_map.span_for_range(self.value.syntax().text_range()),
1439+
"malformed macro invocation",
1440+
)));
14381441
};
14391442

14401443
macro_call_as_call_id_with_eager(

‎crates/hir-def/src/macro_expansion_tests/mbe/regression.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1084,7 +1084,7 @@ fn main() {
10841084
macro_rules! concat_bytes {}
10851085
10861086
fn main() {
1087-
let x = /* error: unexpected token in input */b"";
1087+
let x = /* error: unexpected token */b"";
10881088
}
10891089
10901090
"#]],

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,7 @@ impl DefCollector<'_> {
13241324
self.def_map.diagnostics.push(DefDiagnostic::macro_error(
13251325
directive.module_id,
13261326
ast_id,
1327+
(**path).clone(),
13271328
err,
13281329
));
13291330
return recollect_without(self);

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use std::ops::Not;
44

55
use cfg::{CfgExpr, CfgOptions};
6-
use hir_expand::{attrs::AttrId, ExpandError, MacroCallKind};
6+
use hir_expand::{attrs::AttrId, ExpandErrorKind, MacroCallKind};
77
use la_arena::Idx;
88
use syntax::ast;
99

@@ -25,7 +25,7 @@ pub enum DefDiagnosticKind {
2525
InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },
2626
MalformedDerive { ast: AstId<ast::Adt>, id: usize },
2727
MacroDefError { ast: AstId<ast::Macro>, message: String },
28-
MacroError { ast: AstId<ast::Item>, err: ExpandError },
28+
MacroError { ast: AstId<ast::Item>, path: ModPath, err: ExpandErrorKind },
2929
}
3030

3131
#[derive(Clone, Debug, PartialEq, Eq)]
@@ -82,8 +82,13 @@ impl DefDiagnostic {
8282
Self { in_module: container, kind: DefDiagnosticKind::UnresolvedImport { id, index } }
8383
}
8484

85-
pub fn macro_error(container: LocalModuleId, ast: AstId<ast::Item>, err: ExpandError) -> Self {
86-
Self { in_module: container, kind: DefDiagnosticKind::MacroError { ast, err } }
85+
pub fn macro_error(
86+
container: LocalModuleId,
87+
ast: AstId<ast::Item>,
88+
path: ModPath,
89+
err: ExpandErrorKind,
90+
) -> Self {
91+
Self { in_module: container, kind: DefDiagnosticKind::MacroError { ast, path, err } }
8792
}
8893

8994
pub fn unconfigured_code(

‎crates/hir-expand/src/builtin/derive_macro.rs‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ use crate::{
1212
builtin::quote::{dollar_crate, quote},
1313
db::ExpandDatabase,
1414
hygiene::span_with_def_site_ctxt,
15-
name,
16-
name::{AsName, Name},
15+
name::{self, AsName, Name},
1716
span_map::ExpansionSpanMap,
1817
tt, ExpandError, ExpandResult,
1918
};
@@ -129,13 +128,17 @@ impl VariantShape {
129128
}
130129
}
131130

132-
fn from(tm: &ExpansionSpanMap, value: Option<FieldList>) -> Result<Self, ExpandError> {
131+
fn from(
132+
call_site: Span,
133+
tm: &ExpansionSpanMap,
134+
value: Option<FieldList>,
135+
) -> Result<Self, ExpandError> {
133136
let r = match value {
134137
None => VariantShape::Unit,
135138
Some(FieldList::RecordFieldList(it)) => VariantShape::Struct(
136139
it.fields()
137140
.map(|it| it.name())
138-
.map(|it| name_to_token(tm, it))
141+
.map(|it| name_to_token(call_site, tm, it))
139142
.collect::<Result<_, _>>()?,
140143
),
141144
Some(FieldList::TupleFieldList(it)) => VariantShape::Tuple(it.fields().count()),
@@ -212,16 +215,17 @@ fn parse_adt(tt: &tt::Subtree, call_site: Span) -> Result<BasicAdtInfo, ExpandEr
212215
parser::Edition::CURRENT_FIXME,
213216
);
214217
let macro_items = ast::MacroItems::cast(parsed.syntax_node())
215-
.ok_or_else(|| ExpandError::other("invalid item definition"))?;
216-
let item = macro_items.items().next().ok_or_else(|| ExpandError::other("no item found"))?;
218+
.ok_or_else(|| ExpandError::other(call_site, "invalid item definition"))?;
219+
let item =
220+
macro_items.items().next().ok_or_else(|| ExpandError::other(call_site, "no item found"))?;
217221
let adt = &ast::Adt::cast(item.syntax().clone())
218-
.ok_or_else(|| ExpandError::other("expected struct, enum or union"))?;
222+
.ok_or_else(|| ExpandError::other(call_site, "expected struct, enum or union"))?;
219223
let (name, generic_param_list, where_clause, shape) = match adt {
220224
ast::Adt::Struct(it) => (
221225
it.name(),
222226
it.generic_param_list(),
223227
it.where_clause(),
224-
AdtShape::Struct(VariantShape::from(tm, it.field_list())?),
228+
AdtShape::Struct(VariantShape::from(call_site, tm, it.field_list())?),
225229
),
226230
ast::Adt::Enum(it) => {
227231
let default_variant = it
@@ -241,8 +245,8 @@ fn parse_adt(tt: &tt::Subtree, call_site: Span) -> Result<BasicAdtInfo, ExpandEr
241245
.flat_map(|it| it.variants())
242246
.map(|it| {
243247
Ok((
244-
name_to_token(tm, it.name())?,
245-
VariantShape::from(tm, it.field_list())?,
248+
name_to_token(call_site, tm, it.name())?,
249+
VariantShape::from(call_site, tm, it.field_list())?,
246250
))
247251
})
248252
.collect::<Result<_, ExpandError>>()?,
@@ -357,17 +361,18 @@ fn parse_adt(tt: &tt::Subtree, call_site: Span) -> Result<BasicAdtInfo, ExpandEr
357361
)
358362
})
359363
.collect();
360-
let name_token = name_to_token(tm, name)?;
364+
let name_token = name_to_token(call_site, tm, name)?;
361365
Ok(BasicAdtInfo { name: name_token, shape, param_types, where_clause, associated_types })
362366
}
363367

364368
fn name_to_token(
369+
call_site: Span,
365370
token_map: &ExpansionSpanMap,
366371
name: Option<ast::Name>,
367372
) -> Result<tt::Ident, ExpandError> {
368373
let name = name.ok_or_else(|| {
369374
debug!("parsed item has no name");
370-
ExpandError::other("missing name")
375+
ExpandError::other(call_site, "missing name")
371376
})?;
372377
let span = token_map.span_at(name.syntax().text_range().start());
373378

‎crates/hir-expand/src/builtin/fn_macro.rs‎

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -460,15 +460,11 @@ fn compile_error_expand(
460460
let err = match &*tt.token_trees {
461461
[tt::TokenTree::Leaf(tt::Leaf::Literal(tt::Literal {
462462
symbol: text,
463-
span: _,
463+
span,
464464
kind: tt::LitKind::Str | tt::LitKind::StrRaw(_),
465465
suffix: _,
466-
}))] =>
467-
// FIXME: Use the span here!
468-
{
469-
ExpandError::other(Box::from(unescape_str(text).as_str()))
470-
}
471-
_ => ExpandError::other("`compile_error!` argument must be a string"),
466+
}))] => ExpandError::other(*span, Box::from(unescape_str(text).as_str())),
467+
_ => ExpandError::other(span, "`compile_error!` argument must be a string"),
472468
};
473469

474470
ExpandResult { value: quote! {span =>}, err: Some(err) }
@@ -478,7 +474,7 @@ fn concat_expand(
478474
_db: &dyn ExpandDatabase,
479475
_arg_id: MacroCallId,
480476
tt: &tt::Subtree,
481-
_: Span,
477+
call_site: Span,
482478
) -> ExpandResult<tt::Subtree> {
483479
let mut err = None;
484480
let mut text = String::new();
@@ -527,7 +523,9 @@ fn concat_expand(
527523
| tt::LitKind::ByteStrRaw(_)
528524
| tt::LitKind::CStr
529525
| tt::LitKind::CStrRaw(_)
530-
| tt::LitKind::Err(_) => err = Some(ExpandError::other("unexpected literal")),
526+
| tt::LitKind::Err(_) => {
527+
err = Some(ExpandError::other(it.span, "unexpected literal"))
528+
}
531529
}
532530
}
533531
// handle boolean literals
@@ -539,7 +537,7 @@ fn concat_expand(
539537
}
540538
tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if i % 2 == 1 && punct.char == ',' => (),
541539
_ => {
542-
err.get_or_insert(mbe::ExpandError::UnexpectedToken.into());
540+
err.get_or_insert(ExpandError::other(call_site, "unexpected token"));
543541
}
544542
}
545543
}
@@ -551,7 +549,7 @@ fn concat_bytes_expand(
551549
_db: &dyn ExpandDatabase,
552550
_arg_id: MacroCallId,
553551
tt: &tt::Subtree,
554-
_: Span,
552+
call_site: Span,
555553
) -> ExpandResult<tt::Subtree> {
556554
let mut bytes = String::new();
557555
let mut err = None;
@@ -585,20 +583,22 @@ fn concat_bytes_expand(
585583
bytes.extend(text.as_str().escape_debug());
586584
}
587585
_ => {
588-
err.get_or_insert(mbe::ExpandError::UnexpectedToken.into());
586+
err.get_or_insert(ExpandError::other(*span, "unexpected token"));
589587
break;
590588
}
591589
}
592590
}
593591
tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if i % 2 == 1 && punct.char == ',' => (),
594592
tt::TokenTree::Subtree(tree) if tree.delimiter.kind == tt::DelimiterKind::Bracket => {
595-
if let Err(e) = concat_bytes_expand_subtree(tree, &mut bytes, &mut record_span) {
593+
if let Err(e) =
594+
concat_bytes_expand_subtree(tree, &mut bytes, &mut record_span, call_site)
595+
{
596596
err.get_or_insert(e);
597597
break;
598598
}
599599
}
600600
_ => {
601-
err.get_or_insert(mbe::ExpandError::UnexpectedToken.into());
601+
err.get_or_insert(ExpandError::other(call_site, "unexpected token"));
602602
break;
603603
}
604604
}
@@ -623,6 +623,7 @@ fn concat_bytes_expand_subtree(
623623
tree: &tt::Subtree,
624624
bytes: &mut String,
625625
mut record_span: impl FnMut(Span),
626+
err_span: Span,
626627
) -> Result<(), ExpandError> {
627628
for (ti, tt) in tree.token_trees.iter().enumerate() {
628629
match tt {
@@ -650,7 +651,7 @@ fn concat_bytes_expand_subtree(
650651
}
651652
tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if ti % 2 == 1 && punct.char == ',' => (),
652653
_ => {
653-
return Err(mbe::ExpandError::UnexpectedToken.into());
654+
return Err(ExpandError::other(err_span, "unexpected token"));
654655
}
655656
}
656657
}
@@ -672,7 +673,7 @@ fn concat_idents_expand(
672673
}
673674
tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if i % 2 == 1 && punct.char == ',' => (),
674675
_ => {
675-
err.get_or_insert(mbe::ExpandError::UnexpectedToken.into());
676+
err.get_or_insert(ExpandError::other(span, "unexpected token"));
676677
}
677678
}
678679
}
@@ -686,16 +687,17 @@ fn relative_file(
686687
call_id: MacroCallId,
687688
path_str: &str,
688689
allow_recursion: bool,
690+
err_span: Span,
689691
) -> Result<EditionedFileId, ExpandError> {
690692
let lookup = call_id.lookup(db);
691693
let call_site = lookup.kind.file_id().original_file_respecting_includes(db).file_id();
692694
let path = AnchoredPath { anchor: call_site, path: path_str };
693695
let res = db
694696
.resolve_path(path)
695-
.ok_or_else(|| ExpandError::other(format!("failed to load file `{path_str}`")))?;
697+
.ok_or_else(|| ExpandError::other(err_span, format!("failed to load file `{path_str}`")))?;
696698
// Prevent include itself
697699
if res == call_site && !allow_recursion {
698-
Err(ExpandError::other(format!("recursive inclusion of `{path_str}`")))
700+
Err(ExpandError::other(err_span, format!("recursive inclusion of `{path_str}`")))
699701
} else {
700702
Ok(EditionedFileId::new(res, db.crate_graph()[lookup.krate].edition))
701703
}
@@ -727,7 +729,7 @@ fn parse_string(tt: &tt::Subtree) -> Result<(Symbol, Span), ExpandError> {
727729
}
728730
_ => None,
729731
})
730-
.ok_or(mbe::ExpandError::ConversionError.into())
732+
.ok_or(ExpandError::other(tt.delimiter.open, "expected string literal"))
731733
}
732734

733735
fn include_expand(
@@ -751,7 +753,7 @@ fn include_expand(
751753
Some(it) => ExpandResult::ok(it),
752754
None => ExpandResult::new(
753755
tt::Subtree::empty(DelimSpan { open: span, close: span }),
754-
ExpandError::other("failed to parse included file"),
756+
ExpandError::other(span, "failed to parse included file"),
755757
),
756758
}
757759
}
@@ -761,7 +763,7 @@ pub fn include_input_to_file_id(
761763
arg_id: MacroCallId,
762764
arg: &tt::Subtree,
763765
) -> Result<EditionedFileId, ExpandError> {
764-
relative_file(db, arg_id, parse_string(arg)?.0.as_str(), false)
766+
relative_file(db, arg_id, parse_string(arg)?.0.as_str(), false, arg.delimiter.open)
765767
}
766768

767769
fn include_bytes_expand(
@@ -800,7 +802,7 @@ fn include_str_expand(
800802
// it's unusual to `include_str!` a Rust file), but we can return an empty string.
801803
// Ideally, we'd be able to offer a precise expansion if the user asks for macro
802804
// expansion.
803-
let file_id = match relative_file(db, arg_id, path.as_str(), true) {
805+
let file_id = match relative_file(db, arg_id, path.as_str(), true, span) {
804806
Ok(file_id) => file_id,
805807
Err(_) => {
806808
return ExpandResult::ok(quote!(span =>""));
@@ -836,7 +838,10 @@ fn env_expand(
836838
// The only variable rust-analyzer ever sets is `OUT_DIR`, so only diagnose that to avoid
837839
// unnecessary diagnostics for eg. `CARGO_PKG_NAME`.
838840
if key.as_str() == "OUT_DIR" {
839-
err = Some(ExpandError::other(r#"`OUT_DIR` not set, enable "build scripts" to fix"#));
841+
err = Some(ExpandError::other(
842+
span,
843+
r#"`OUT_DIR` not set, enable "build scripts" to fix"#,
844+
));
840845
}
841846

842847
// If the variable is unset, still return a dummy string to help type inference along.
@@ -885,7 +890,7 @@ fn quote_expand(
885890
) -> ExpandResult<tt::Subtree> {
886891
ExpandResult::new(
887892
tt::Subtree::empty(tt::DelimSpan { open: span, close: span }),
888-
ExpandError::other("quote! is not implemented"),
893+
ExpandError::other(span, "quote! is not implemented"),
889894
)
890895
}
891896

0 commit comments

Comments
 (0)