rust-analyzer

diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs
index 928aadfbcc..f4304ae7e8 100644
--- a/crates/hir-def/src/body.rs
+++ b/crates/hir-def/src/body.rs

@@ -138,6 +138,7 @@ impl Expander {

db: &dyn DefDatabase,

macro_call: ast::MacroCall,

) -> Result<ExpandResult<Option<(Mark, T)>>, UnresolvedMacro> {

+ // FIXME: within_limit should support this, instead of us having to extract the error

let mut unresolved_macro_err = None;

let result = self.within_limit(db, |this| {

@@ -146,22 +147,13 @@ impl Expander {

let resolver =

|path| this.resolve_path_as_macro(db, &path).map(|it| macro_id_to_def_id(db, it));

- let mut err = None;

- let call_id = match macro_call.as_call_id_with_errors(

- db,

- this.def_map.krate(),

- resolver,

- &mut |e| {

- err.get_or_insert(e);

- },

- ) {

+ match macro_call.as_call_id_with_errors(db, this.def_map.krate(), resolver) {

Ok(call_id) => call_id,

Err(resolve_err) => {

unresolved_macro_err = Some(resolve_err);

- return ExpandResult { value: None, err: None };

+ ExpandResult { value: None, err: None }

}

- };

- ExpandResult { value: call_id.ok(), err }

+ }

});

if let Some(err) = unresolved_macro_err {

diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs
index 5a0c1b66b9..55e1e91220 100644
--- a/crates/hir-def/src/lib.rs
+++ b/crates/hir-def/src/lib.rs

@@ -65,11 +65,11 @@ use hir_expand::{

builtin_attr_macro::BuiltinAttrExpander,

builtin_derive_macro::BuiltinDeriveExpander,

builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander},

- eager::{expand_eager_macro, ErrorEmitted, ErrorSink},

+ eager::expand_eager_macro,

hygiene::Hygiene,

proc_macro::ProcMacroExpander,

- AstId, ExpandError, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId,

- MacroDefKind, UnresolvedMacro,

+ AstId, ExpandError, ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind,

+ MacroDefId, MacroDefKind, UnresolvedMacro,

};

use item_tree::ExternBlock;

use la_arena::Idx;

@@ -795,7 +795,7 @@ pub trait AsMacroCall {

krate: CrateId,

resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,

) -> Option<MacroCallId> {

- self.as_call_id_with_errors(db, krate, resolver, &mut |_| ()).ok()?.ok()

+ self.as_call_id_with_errors(db, krate, resolver).ok()?.value

}

fn as_call_id_with_errors(

@@ -803,8 +803,7 @@ pub trait AsMacroCall {

db: &dyn db::DefDatabase,

krate: CrateId,

resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,

- error_sink: &mut dyn FnMut(ExpandError),

- ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro>;

+ ) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro>;

}

impl AsMacroCall for InFile<&ast::MacroCall> {

@@ -813,21 +812,15 @@ impl AsMacroCall for InFile<&ast::MacroCall> {

db: &dyn db::DefDatabase,

krate: CrateId,

resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,

- mut error_sink: &mut dyn FnMut(ExpandError),

- ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {

+ ) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {

let expands_to = hir_expand::ExpandTo::from_call_site(self.value);

let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));

let h = Hygiene::new(db.upcast(), self.file_id);

let path =

self.value.path().and_then(|path| path::ModPath::from_src(db.upcast(), path, &h));

- let path = match error_sink

- .option(path, || ExpandError::Other("malformed macro invocation".into()))

- {

- Ok(path) => path,

- Err(error) => {

- return Ok(Err(error));

- }

+ let Some(path) = path else {

+ return Ok(ExpandResult::only_err(ExpandError::Other("malformed macro invocation".into())));

};

macro_call_as_call_id(

@@ -836,7 +829,6 @@ impl AsMacroCall for InFile<&ast::MacroCall> {

expands_to,

krate,

resolver,

- error_sink,

)

}

@@ -860,21 +852,23 @@ fn macro_call_as_call_id(

expand_to: ExpandTo,

krate: CrateId,

resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,

- error_sink: &mut dyn FnMut(ExpandError),

-) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {

+) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {

let def =

resolver(call.path.clone()).ok_or_else(|| UnresolvedMacro { path: call.path.clone() })?;

let res = if let MacroDefKind::BuiltInEager(..) = def.kind {

let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db.upcast()));

- expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver, error_sink)?

+ expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver)?

} else {

- Ok(def.as_lazy_macro(

- db.upcast(),

- krate,

- MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },

- ))

+ ExpandResult {

+ value: Some(def.as_lazy_macro(

+ db.upcast(),

+ krate,

+ MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },

+ )),

+ err: None,

+ }

};

Ok(res)

}

diff --git a/crates/hir-def/src/macro_expansion_tests/mod.rs b/crates/hir-def/src/macro_expansion_tests/mod.rs
index 314bf22b95..73a495d89b 100644
--- a/crates/hir-def/src/macro_expansion_tests/mod.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mod.rs

@@ -125,21 +125,15 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream

for macro_call in source_file.syntax().descendants().filter_map(ast::MacroCall::cast) {

let macro_call = InFile::new(source.file_id, &macro_call);

- let mut error = None;

- let macro_call_id = macro_call

- .as_call_id_with_errors(

- &db,

- krate,

- |path| {

- resolver.resolve_path_as_macro(&db, &path).map(|it| macro_id_to_def_id(&db, it))

- },

- &mut |err| error = Some(err),

- )

- .unwrap()

+ let res = macro_call

+ .as_call_id_with_errors(&db, krate, |path| {

+ resolver.resolve_path_as_macro(&db, &path).map(|it| macro_id_to_def_id(&db, it))

+ })

.unwrap();

+ let macro_call_id = res.value.unwrap();

let macro_file = MacroFile { macro_call_id };

let mut expansion_result = db.parse_macro_expansion(macro_file);

- expansion_result.err = expansion_result.err.or(error);

+ expansion_result.err = expansion_result.err.or(res.err);

expansions.push((macro_call.value.clone(), expansion_result, db.macro_arg(macro_call_id)));

}

diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 1d625fa3c7..8ab0b3dbd1 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs

@@ -16,8 +16,8 @@ use hir_expand::{

builtin_fn_macro::find_builtin_macro,

name::{name, AsName, Name},

proc_macro::ProcMacroExpander,

- ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc, MacroDefId,

- MacroDefKind,

+ ExpandResult, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroCallLoc,

+ MacroDefId, MacroDefKind,

};

use itertools::{izip, Itertools};

use la_arena::Idx;

@@ -1116,9 +1116,8 @@ impl DefCollector<'_> {

*expand_to,

self.def_map.krate,

resolver_def_id,

- &mut |_err| (),

);

- if let Ok(Ok(call_id)) = call_id {

+ if let Ok(ExpandResult { value: Some(call_id), .. }) = call_id {

push_resolved(directive, call_id);

res = ReachedFixedPoint::No;

return false;

@@ -1414,7 +1413,6 @@ impl DefCollector<'_> {

.take_macros()

.map(|it| macro_id_to_def_id(self.db, it))

- &mut |_| (),

);

if let Err(UnresolvedMacro { path }) = macro_call_as_call_id {

self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(

@@ -2112,7 +2110,6 @@ impl ModCollector<'_, '_> {

let ast_id = AstIdWithPath::new(self.file_id(), mac.ast_id, ModPath::clone(&mac.path));

// Case 1: try to resolve in legacy scope and expand macro_rules

- let mut error = None;

match macro_call_as_call_id(

self.def_collector.db,

&ast_id,

@@ -2133,21 +2130,20 @@ impl ModCollector<'_, '_> {

)

})

- &mut |err| {

- error.get_or_insert(err);

- },

) {

- Ok(Ok(macro_call_id)) => {

+ Ok(res) => {

// Legacy macros need to be expanded immediately, so that any macros they produce

// are in scope.

- self.def_collector.collect_macro_expansion(

- self.module_id,

- macro_call_id,

- self.macro_depth + 1,

- container,

- );

+ if let Some(val) = res.value {

+ self.def_collector.collect_macro_expansion(

+ self.module_id,

+ val,

+ self.macro_depth + 1,

+ container,

+ );

+ }

- if let Some(err) = error {

+ if let Some(err) = res.err {

self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error(

self.module_id,

MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to: mac.expand_to },

@@ -2157,16 +2153,6 @@ impl ModCollector<'_, '_> {

return;

}

- Ok(Err(_)) => {

- // Built-in macro failed eager expansion.

- self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error(

- self.module_id,

- MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to: mac.expand_to },

- error.unwrap().to_string(),

- ));

- return;

- }

Err(UnresolvedMacro { .. }) => (),

}

diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
index d93f3b08d3..d7aff6b02f 100644
--- a/crates/hir-expand/src/db.rs
+++ b/crates/hir-expand/src/db.rs

@@ -279,25 +279,28 @@ fn parse_macro_expansion(

let mbe::ValueResult { value, err } = db.macro_expand(macro_file.macro_call_id);

if let Some(err) = &err {

- // Note:

- // The final goal we would like to make all parse_macro success,

- // such that the following log will not call anyway.

- let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);

- let node = loc.kind.to_node(db);

- // collect parent information for warning log

- let parents =

- std::iter::successors(loc.kind.file_id().call_node(db), |it| it.file_id.call_node(db))

- .map(|n| format!("{:#}", n.value))

- .collect::<Vec<_>>()

- .join("\n");

- tracing::debug!(

- "fail on macro_parse: (reason: {:?} macro_call: {:#}) parents: {}",

- err,

- node.value,

- parents

- );

+ if tracing::enabled!(tracing::Level::DEBUG) {

+ // Note:

+ // The final goal we would like to make all parse_macro success,

+ // such that the following log will not call anyway.

+ let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);

+ let node = loc.kind.to_node(db);

+ // collect parent information for warning log

+ let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| {

+ it.file_id.call_node(db)

+ })

+ .map(|n| format!("{:#}", n.value))

+ .collect::<Vec<_>>()

+ .join("\n");

+ tracing::debug!(

+ "fail on macro_parse: (reason: {:?} macro_call: {:#}) parents: {}",

+ err,

+ node.value,

+ parents

+ );

+ }

}

let tt = match value {

Some(tt) => tt,

@@ -466,7 +469,8 @@ fn macro_expand(

Ok(it) => it,

// FIXME: This is weird -- we effectively report macro *definition*

// errors lazily, when we try to expand the macro. Instead, they should

- // be reported at the definition site (when we construct a def map).

+ // be reported at the definition site when we construct a def map.

+ // (Note we do report them also at the definition site in the late diagnostic pass)

Err(err) => {

return ExpandResult::only_err(ExpandError::Other(

format!("invalid macro definition: {err}").into(),

diff --git a/crates/hir-expand/src/eager.rs b/crates/hir-expand/src/eager.rs
index 6b646f5e4f..74adced8c6 100644
--- a/crates/hir-expand/src/eager.rs
+++ b/crates/hir-expand/src/eager.rs

@@ -32,77 +32,16 @@ use crate::{

MacroCallLoc, MacroDefId, MacroDefKind, UnresolvedMacro,

};

-#[derive(Debug)]

-pub struct ErrorEmitted {

- _private: (),

-pub trait ErrorSink {

- fn emit(&mut self, err: ExpandError);

- fn option<T>(

- &mut self,

- opt: Option<T>,

- error: impl FnOnce() -> ExpandError,

- ) -> Result<T, ErrorEmitted> {

- match opt {

- Some(it) => Ok(it),

- None => {

- self.emit(error());

- Err(ErrorEmitted { _private: () })

- }

- fn option_with<T>(

- &mut self,

- opt: impl FnOnce() -> Option<T>,

- error: impl FnOnce() -> ExpandError,

- ) -> Result<T, ErrorEmitted> {

- self.option(opt(), error)

- }

- fn result<T>(&mut self, res: Result<T, ExpandError>) -> Result<T, ErrorEmitted> {

- match res {

- Ok(it) => Ok(it),

- Err(e) => {

- self.emit(e);

- Err(ErrorEmitted { _private: () })

- }

- fn expand_result_option<T>(&mut self, res: ExpandResult<Option<T>>) -> Result<T, ErrorEmitted> {

- match (res.value, res.err) {

- (None, Some(err)) => {

- self.emit(err);

- Err(ErrorEmitted { _private: () })

- }

- (Some(value), opt_err) => {

- if let Some(err) = opt_err {

- self.emit(err);

- }

- Ok(value)

- }

- (None, None) => unreachable!("`ExpandResult` without value or error"),

- }

-impl ErrorSink for &'_ mut dyn FnMut(ExpandError) {

- fn emit(&mut self, err: ExpandError) {

- self(err);

- }

pub fn expand_eager_macro(

db: &dyn ExpandDatabase,

krate: CrateId,

macro_call: InFile<ast::MacroCall>,

def: MacroDefId,

resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,

- diagnostic_sink: &mut dyn FnMut(ExpandError),

-) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {

+) -> Result<ExpandResult<Option<MacroCallId>>, UnresolvedMacro> {

+ let MacroDefKind::BuiltInEager(eager, _) = def.kind else {

+ panic!("called `expand_eager_macro` on non-eager macro def {def:?}")

+ };

let hygiene = Hygiene::new(db, macro_call.file_id);

let parsed_args = macro_call

.value

@@ -129,40 +68,34 @@ pub fn expand_eager_macro(

});

let parsed_args = mbe::token_tree_to_syntax_node(&parsed_args, mbe::TopEntryPoint::Expr).0;

- let result = match eager_macro_recur(

+ let ExpandResult { value, mut err } = eager_macro_recur(

db,

&hygiene,

InFile::new(arg_id.as_file(), parsed_args.syntax_node()),

krate,

resolver,

- diagnostic_sink,

- ) {

- Ok(Ok(it)) => it,

- Ok(Err(err)) => return Ok(Err(err)),

- Err(err) => return Err(err),

+ )?;

+ let Some(value ) = value else {

+ return Ok(ExpandResult { value: None, err })

};

- let subtree = to_subtree(&result);

+ let subtree = to_subtree(&value);

- if let MacroDefKind::BuiltInEager(eager, _) = def.kind {

- let res = eager.expand(db, arg_id, &subtree);

- if let Some(err) = res.err {

- diagnostic_sink(err);

- }

+ let res = eager.expand(db, arg_id, &subtree);

+ if err.is_none() {

+ err = res.err;

+ }

- let loc = MacroCallLoc {

- def,

- krate,

- eager: Some(EagerCallInfo {

- arg_or_expansion: Arc::new(res.value.subtree),

- included_file: res.value.included_file,

- }),

- kind: MacroCallKind::FnLike { ast_id: call_id, expand_to },

- };

+ let loc = MacroCallLoc {

+ def,

+ krate,

+ eager: Some(EagerCallInfo {

+ arg_or_expansion: Arc::new(res.value.subtree),

+ included_file: res.value.included_file,

+ }),

+ kind: MacroCallKind::FnLike { ast_id: call_id, expand_to },

+ };

- Ok(Ok(db.intern_macro_call(loc)))

- } else {

- panic!("called `expand_eager_macro` on non-eager macro def {def:?}");

- }

+ Ok(ExpandResult { value: Some(db.intern_macro_call(loc)), err })

}

fn to_subtree(node: &SyntaxNode) -> crate::tt::Subtree {

@@ -201,23 +134,25 @@ fn eager_macro_recur(

curr: InFile<SyntaxNode>,

krate: CrateId,

macro_resolver: &dyn Fn(ModPath) -> Option<MacroDefId>,

- mut diagnostic_sink: &mut dyn FnMut(ExpandError),

-) -> Result<Result<SyntaxNode, ErrorEmitted>, UnresolvedMacro> {

+) -> Result<ExpandResult<Option<SyntaxNode>>, UnresolvedMacro> {

let original = curr.value.clone_for_update();

let children = original.descendants().filter_map(ast::MacroCall::cast);

let mut replacements = Vec::new();

+ // Note: We only report a single error inside of eager expansions

+ let mut error = None;

// Collect replacement

for child in children {

let def = match child.path().and_then(|path| ModPath::from_src(db, path, hygiene)) {

Some(path) => macro_resolver(path.clone()).ok_or(UnresolvedMacro { path })?,

None => {

- diagnostic_sink(ExpandError::Other("malformed macro invocation".into()));

+ error = Some(ExpandError::Other("malformed macro invocation".into()));

continue;

}

};

- let insert = match def.kind {

+ let ExpandResult { value, err } = match def.kind {

MacroDefKind::BuiltInEager(..) => {

let id = match expand_eager_macro(

db,

@@ -225,45 +160,49 @@ fn eager_macro_recur(

curr.with_value(child.clone()),

def,

macro_resolver,

- diagnostic_sink,

) {

- Ok(Ok(it)) => it,

- Ok(Err(err)) => return Ok(Err(err)),

+ Ok(it) => it,

Err(err) => return Err(err),

};

- db.parse_or_expand(id.as_file())

- .expect("successful macro expansion should be parseable")

- .clone_for_update()

+ id.map(|call| {

+ call.and_then(|call| db.parse_or_expand(call.as_file()))

+ .map(|it| it.clone_for_update())

+ })

}

MacroDefKind::Declarative(_)

| MacroDefKind::BuiltIn(..)

| MacroDefKind::BuiltInAttr(..)

| MacroDefKind::BuiltInDerive(..)

| MacroDefKind::ProcMacro(..) => {

- let res = lazy_expand(db, &def, curr.with_value(child.clone()), krate);

- let val = match diagnostic_sink.expand_result_option(res) {

- Ok(it) => it,

- Err(err) => return Ok(Err(err)),

- };

- // replace macro inside

- let hygiene = Hygiene::new(db, val.file_id);

- match eager_macro_recur(db, &hygiene, val, krate, macro_resolver, diagnostic_sink) {

- Ok(Ok(it)) => it,

- Ok(Err(err)) => return Ok(Err(err)),

- Err(err) => return Err(err),

+ let ExpandResult { value, err } =

+ lazy_expand(db, &def, curr.with_value(child.clone()), krate);

+ match value {

+ Some(val) => {

+ // replace macro inside

+ let hygiene = Hygiene::new(db, val.file_id);

+ let ExpandResult { value, err: error } =

+ eager_macro_recur(db, &hygiene, val, krate, macro_resolver)?;

+ let err = if err.is_none() { error } else { err };

+ ExpandResult { value, err }

+ }

+ None => ExpandResult { value: None, err },

}

};

+ if err.is_some() {

+ error = err;

+ }

// check if the whole original syntax is replaced

if child.syntax() == &original {

- return Ok(Ok(insert));

+ return Ok(ExpandResult { value, err: error });

}

- replacements.push((child, insert));

+ if let Some(insert) = value {

+ replacements.push((child, insert));

+ }

}

replacements.into_iter().rev().for_each(|(old, new)| ted::replace(old.syntax(), new));

- Ok(Ok(original))

+ Ok(ExpandResult { value: Some(original), err: error })

}