Skip to content

Commit 722f79d

Browse files
committed
Auto merge of #17707 - Veykril:proc-macro-err-cleanup, r=Veykril
feat: Use spans for builtin and declarative macro expansion errors This should generally improve some error reporting for macro expansion errors. Especially for `compile_error!` within proc-macros
2 parents 51130d8 + 5ac8b79 commit 722f79d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+771
-826
lines changed

src/tools/rust-analyzer/crates/base-db/src/input.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ use span::{Edition, EditionedFileId};
1616
use triomphe::Arc;
1717
use vfs::{file_set::FileSet, AbsPathBuf, AnchoredPath, FileId, VfsPath};
1818

19-
// Map from crate id to the name of the crate and path of the proc-macro. If the value is `None`,
20-
// then the crate for the proc-macro hasn't been build yet as the build data is missing.
21-
pub type ProcMacroPaths = FxHashMap<CrateId, Result<(Option<String>, AbsPathBuf), String>>;
19+
pub type ProcMacroPaths = FxHashMap<CrateId, Result<(String, AbsPathBuf), String>>;
2220

2321
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
2422
pub struct SourceRootId(pub u32);

src/tools/rust-analyzer/crates/hir-def/src/body.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::ops::{Deref, Index};
1010

1111
use base_db::CrateId;
1212
use cfg::{CfgExpr, CfgOptions};
13-
use hir_expand::{name::Name, InFile};
13+
use hir_expand::{name::Name, ExpandError, InFile};
1414
use la_arena::{Arena, ArenaMap, Idx, RawIdx};
1515
use rustc_hash::FxHashMap;
1616
use smallvec::SmallVec;
@@ -115,8 +115,7 @@ pub struct SyntheticSyntax;
115115
#[derive(Debug, Eq, PartialEq)]
116116
pub enum BodyDiagnostic {
117117
InactiveCode { node: InFile<SyntaxNodePtr>, cfg: CfgExpr, opts: CfgOptions },
118-
MacroError { node: InFile<AstPtr<ast::MacroCall>>, message: String },
119-
UnresolvedProcMacro { node: InFile<AstPtr<ast::MacroCall>>, krate: CrateId },
118+
MacroError { node: InFile<AstPtr<ast::MacroCall>>, err: ExpandError },
120119
UnresolvedMacroCall { node: InFile<AstPtr<ast::MacroCall>>, path: ModPath },
121120
UnreachableLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name },
122121
UndeclaredLabel { node: InFile<AstPtr<ast::Lifetime>>, name: Name },

src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use base_db::CrateId;
77
use either::Either;
88
use hir_expand::{
99
name::{AsName, Name},
10-
ExpandError, InFile,
10+
InFile,
1111
};
1212
use intern::{sym, Interned, Symbol};
1313
use rustc_hash::FxHashMap;
@@ -992,20 +992,11 @@ impl ExprCollector<'_> {
992992
}
993993
};
994994
if record_diagnostics {
995-
match &res.err {
996-
Some(ExpandError::UnresolvedProcMacro(krate)) => {
997-
self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro {
998-
node: InFile::new(outer_file, syntax_ptr),
999-
krate: *krate,
1000-
});
1001-
}
1002-
Some(err) => {
1003-
self.source_map.diagnostics.push(BodyDiagnostic::MacroError {
1004-
node: InFile::new(outer_file, syntax_ptr),
1005-
message: err.to_string(),
1006-
});
1007-
}
1008-
None => {}
995+
if let Some(err) = res.err {
996+
self.source_map.diagnostics.push(BodyDiagnostic::MacroError {
997+
node: InFile::new(outer_file, syntax_ptr),
998+
err,
999+
});
10091000
}
10101001
}
10111002

src/tools/rust-analyzer/crates/hir-def/src/data.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -657,22 +657,18 @@ impl<'a> AssocItemCollector<'a> {
657657
// crate failed), skip expansion like we would if it was
658658
// disabled. This is analogous to the handling in
659659
// `DefCollector::collect_macros`.
660-
if exp.is_dummy() {
661-
self.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
660+
if let Some(err) = exp.as_expand_error(self.module_id.krate) {
661+
self.diagnostics.push(DefDiagnostic::macro_error(
662662
self.module_id.local_id,
663-
loc.kind,
664-
loc.def.krate,
663+
ast_id,
664+
(*attr.path).clone(),
665+
err,
665666
));
666-
667-
continue 'attrs;
668-
}
669-
if exp.is_disabled() {
670667
continue 'attrs;
671668
}
672669
}
673670

674671
self.macro_calls.push((ast_id, call_id));
675-
676672
let res =
677673
self.expander.enter_expand_id::<ast::MacroItems>(self.db, call_id);
678674
self.collect_macro_items(res);

src/tools/rust-analyzer/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::UnresolvedProcMacro(_)) => None,
186+
Some(e) if matches!(e.kind(), ExpandErrorKind::MissingProcMacroExpander(_)) => None,
183187
_ => (|| {
184188
let parse = res.value.0.cast::<T>()?;
185189

src/tools/rust-analyzer/crates/hir-def/src/lib.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,7 @@ use base_db::{
7575
CrateId,
7676
};
7777
use hir_expand::{
78-
builtin_attr_macro::BuiltinAttrExpander,
79-
builtin_derive_macro::BuiltinDeriveExpander,
80-
builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander},
78+
builtin::{BuiltinAttrExpander, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerExpander},
8179
db::ExpandDatabase,
8280
eager::expand_eager_macro_input,
8381
impl_intern_lookup,
@@ -1436,7 +1434,10 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
14361434
});
14371435

14381436
let Some((call_site, path)) = path else {
1439-
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+
)));
14401441
};
14411442

14421443
macro_call_as_call_id_with_eager(

src/tools/rust-analyzer/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
"#]],

src/tools/rust-analyzer/crates/hir-def/src/macro_expansion_tests/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
122122

123123
let mut expn_text = String::new();
124124
if let Some(err) = exp.err {
125-
format_to!(expn_text, "/* error: {} */", err);
125+
format_to!(expn_text, "/* error: {} */", err.render_to_string(&db).0);
126126
}
127127
let (parse, token_map) = exp.value;
128128
if expect_errors {

src/tools/rust-analyzer/crates/hir-def/src/nameres.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,6 @@ struct DefMapCrateData {
145145
/// Side table for resolving derive helpers.
146146
exported_derives: FxHashMap<MacroDefId, Box<[Name]>>,
147147
fn_proc_macro_mapping: FxHashMap<FunctionId, ProcMacroId>,
148-
/// The error that occurred when failing to load the proc-macro dll.
149-
proc_macro_loading_error: Option<Box<str>>,
150148

151149
/// Custom attributes registered with `#![register_attr]`.
152150
registered_attrs: Vec<Symbol>,
@@ -169,7 +167,6 @@ impl DefMapCrateData {
169167
extern_prelude: FxIndexMap::default(),
170168
exported_derives: FxHashMap::default(),
171169
fn_proc_macro_mapping: FxHashMap::default(),
172-
proc_macro_loading_error: None,
173170
registered_attrs: Vec::new(),
174171
registered_tools: PREDEFINED_TOOLS.iter().map(|it| Symbol::intern(it)).collect(),
175172
unstable_features: FxHashSet::default(),
@@ -189,7 +186,6 @@ impl DefMapCrateData {
189186
registered_attrs,
190187
registered_tools,
191188
unstable_features,
192-
proc_macro_loading_error: _,
193189
rustc_coherence_is_core: _,
194190
no_core: _,
195191
no_std: _,
@@ -474,10 +470,6 @@ impl DefMap {
474470
self.data.fn_proc_macro_mapping.get(&id).copied()
475471
}
476472

477-
pub fn proc_macro_loading_error(&self) -> Option<&str> {
478-
self.data.proc_macro_loading_error.as_deref()
479-
}
480-
481473
pub fn krate(&self) -> CrateId {
482474
self.krate
483475
}

src/tools/rust-analyzer/crates/hir-def/src/nameres/collector.rs

Lines changed: 27 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@ use cfg::{CfgExpr, CfgOptions};
1010
use either::Either;
1111
use hir_expand::{
1212
attrs::{Attr, AttrId},
13-
builtin_attr_macro::find_builtin_attr,
14-
builtin_derive_macro::find_builtin_derive,
15-
builtin_fn_macro::find_builtin_macro,
13+
builtin::{find_builtin_attr, find_builtin_derive, find_builtin_macro},
1614
name::{AsName, Name},
1715
proc_macro::CustomProcMacroExpander,
1816
ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
@@ -76,34 +74,11 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
7674
}
7775

7876
let proc_macros = if krate.is_proc_macro {
79-
match db.proc_macros().get(&def_map.krate) {
80-
Some(Ok(proc_macros)) => Ok({
81-
let ctx = db.syntax_context(tree_id.file_id());
82-
proc_macros
83-
.iter()
84-
.enumerate()
85-
.map(|(idx, it)| {
86-
let name = Name::new_symbol(it.name.clone(), ctx);
87-
(
88-
name,
89-
if !db.expand_proc_attr_macros() {
90-
CustomProcMacroExpander::dummy()
91-
} else if it.disabled {
92-
CustomProcMacroExpander::disabled()
93-
} else {
94-
CustomProcMacroExpander::new(
95-
hir_expand::proc_macro::ProcMacroId::new(idx as u32),
96-
)
97-
},
98-
)
99-
})
100-
.collect()
101-
}),
102-
Some(Err(e)) => Err(e.clone().into_boxed_str()),
103-
None => Err("No proc-macros present for crate".to_owned().into_boxed_str()),
104-
}
77+
db.proc_macros()
78+
.for_crate(def_map.krate, db.syntax_context(tree_id.file_id()))
79+
.unwrap_or_default()
10580
} else {
106-
Ok(vec![])
81+
Default::default()
10782
};
10883

10984
let mut collector = DefCollector {
@@ -252,10 +227,10 @@ struct DefCollector<'a> {
252227
mod_dirs: FxHashMap<LocalModuleId, ModDir>,
253228
cfg_options: &'a CfgOptions,
254229
/// List of procedural macros defined by this crate. This is read from the dynamic library
255-
/// built by the build system, and is the list of proc. macros we can actually expand. It is
256-
/// empty when proc. macro support is disabled (in which case we still do name resolution for
257-
/// them).
258-
proc_macros: Result<Vec<(Name, CustomProcMacroExpander)>, Box<str>>,
230+
/// built by the build system, and is the list of proc-macros we can actually expand. It is
231+
/// empty when proc-macro support is disabled (in which case we still do name resolution for
232+
/// them). The bool signals whether the proc-macro has been explicitly disabled for name-resolution.
233+
proc_macros: Box<[(Name, CustomProcMacroExpander, bool)]>,
259234
is_proc_macro: bool,
260235
from_glob_import: PerNsGlobImports,
261236
/// If we fail to resolve an attribute on a `ModItem`, we fall back to ignoring the attribute.
@@ -278,10 +253,6 @@ impl DefCollector<'_> {
278253
let attrs = item_tree.top_level_attrs(self.db, self.def_map.krate);
279254
let crate_data = Arc::get_mut(&mut self.def_map.data).unwrap();
280255

281-
if let Err(e) = &self.proc_macros {
282-
crate_data.proc_macro_loading_error = Some(e.clone());
283-
}
284-
285256
let mut process = true;
286257

287258
// Process other crate-level attributes.
@@ -608,11 +579,17 @@ impl DefCollector<'_> {
608579
fn_id: FunctionId,
609580
) {
610581
let kind = def.kind.to_basedb_kind();
611-
let (expander, kind) =
612-
match self.proc_macros.as_ref().map(|it| it.iter().find(|(n, _)| n == &def.name)) {
613-
Ok(Some(&(_, expander))) => (expander, kind),
614-
_ => (CustomProcMacroExpander::dummy(), kind),
615-
};
582+
let (expander, kind) = match self.proc_macros.iter().find(|(n, _, _)| n == &def.name) {
583+
Some(_)
584+
if kind == hir_expand::proc_macro::ProcMacroKind::Attr
585+
&& !self.db.expand_proc_attr_macros() =>
586+
{
587+
(CustomProcMacroExpander::disabled_proc_attr(), kind)
588+
}
589+
Some(&(_, _, true)) => (CustomProcMacroExpander::disabled(), kind),
590+
Some(&(_, expander, false)) => (expander, kind),
591+
None => (CustomProcMacroExpander::missing_expander(), kind),
592+
};
616593

617594
let proc_macro_id = ProcMacroLoc {
618595
container: self.def_map.crate_root(),
@@ -1415,25 +1392,23 @@ impl DefCollector<'_> {
14151392
return recollect_without(self);
14161393
}
14171394

1418-
let call_id = call_id();
14191395
if let MacroDefKind::ProcMacro(_, exp, _) = def.kind {
14201396
// If there's no expander for the proc macro (e.g.
14211397
// because proc macros are disabled, or building the
14221398
// proc macro crate failed), report this and skip
14231399
// expansion like we would if it was disabled
1424-
if exp.is_dummy() {
1425-
self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
1400+
if let Some(err) = exp.as_expand_error(def.krate) {
1401+
self.def_map.diagnostics.push(DefDiagnostic::macro_error(
14261402
directive.module_id,
1427-
self.db.lookup_intern_macro_call(call_id).kind,
1428-
def.krate,
1403+
ast_id,
1404+
(**path).clone(),
1405+
err,
14291406
));
14301407
return recollect_without(self);
14311408
}
1432-
if exp.is_disabled() {
1433-
return recollect_without(self);
1434-
}
14351409
}
14361410

1411+
let call_id = call_id();
14371412
self.def_map.modules[directive.module_id]
14381413
.scope
14391414
.add_attr_macro_invoc(ast_id, call_id);
@@ -1472,7 +1447,6 @@ impl DefCollector<'_> {
14721447
}
14731448
let file_id = macro_call_id.as_file();
14741449

1475-
// Then, fetch and process the item tree. This will reuse the expansion result from above.
14761450
let item_tree = self.db.file_item_tree(file_id);
14771451

14781452
let mod_dir = if macro_call_id.as_macro_file().is_include_macro(self.db.upcast()) {
@@ -2510,7 +2484,7 @@ mod tests {
25102484
unresolved_macros: Vec::new(),
25112485
mod_dirs: FxHashMap::default(),
25122486
cfg_options: &CfgOptions::default(),
2513-
proc_macros: Ok(vec![]),
2487+
proc_macros: Default::default(),
25142488
from_glob_import: Default::default(),
25152489
skip_attrs: Default::default(),
25162490
is_proc_macro: false,

0 commit comments

Comments
 (0)