rust-analyzer

diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs
index 29ac666277..8a589a6cf1 100644
--- a/crates/hir-def/src/body/lower.rs
+++ b/crates/hir-def/src/body/lower.rs

@@ -1000,10 +1000,6 @@ impl ExprCollector<'_> {

krate: *krate,

});

}

- Some(ExpandError::RecursionOverflowPoisoned) => {

- // Recursion limit has been reached in the macro expansion tree, but not in

- // this very macro call. Don't add diagnostics to avoid duplication.

- }

Some(err) => {

self.source_map.diagnostics.push(BodyDiagnostic::MacroError {

node: InFile::new(outer_file, syntax_ptr),

diff --git a/crates/hir-def/src/expander.rs b/crates/hir-def/src/expander.rs
index b83feeedc3..b99df1ed59 100644
--- a/crates/hir-def/src/expander.rs
+++ b/crates/hir-def/src/expander.rs

@@ -140,13 +140,11 @@ impl Expander {

// The overflow error should have been reported when it occurred (see the next branch),

// so don't return overflow error here to avoid diagnostics duplication.

cov_mark::hit!(overflow_but_not_me);

- return ExpandResult::only_err(ExpandError::RecursionOverflowPoisoned);

+ return ExpandResult::ok(None);

} else if self.recursion_limit.check(self.recursion_depth as usize + 1).is_err() {

self.recursion_depth = u32::MAX;

cov_mark::hit!(your_stack_belongs_to_me);

- return ExpandResult::only_err(ExpandError::other(

- "reached recursion limit during macro expansion",

- ));

+ return ExpandResult::only_err(ExpandError::RecursionOverflow);

}

let ExpandResult { value, err } = op(self);

diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs b/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs
index 0909d8c835..63f211022c 100644
--- a/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mbe/matching.rs

@@ -33,7 +33,7 @@ m!(&k");

"#,

expect![[r#"

macro_rules! m { ($i:literal) => {}; }

-/* error: invalid token tree */"#]],

+/* error: mismatched delimiters */"#]],

);

}

diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/meta_syntax.rs b/crates/hir-def/src/macro_expansion_tests/mbe/meta_syntax.rs
index e875950e4e..2d289b7683 100644
--- a/crates/hir-def/src/macro_expansion_tests/mbe/meta_syntax.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mbe/meta_syntax.rs

@@ -68,26 +68,26 @@ m2!();

"#,

expect![[r#"

macro_rules! i1 { invalid }

-/* error: invalid macro definition: expected subtree */

+/* error: macro definition has parse errors */

macro_rules! e1 { $i:ident => () }

-/* error: invalid macro definition: expected subtree */

+/* error: macro definition has parse errors */

macro_rules! e2 { ($i:ident) () }

-/* error: invalid macro definition: expected `=` */

+/* error: macro definition has parse errors */

macro_rules! e3 { ($(i:ident)_) => () }

-/* error: invalid macro definition: invalid repeat */

+/* error: macro definition has parse errors */

macro_rules! f1 { ($i) => ($i) }

-/* error: invalid macro definition: missing fragment specifier */

+/* error: macro definition has parse errors */

macro_rules! f2 { ($i:) => ($i) }

-/* error: invalid macro definition: missing fragment specifier */

+/* error: macro definition has parse errors */

macro_rules! f3 { ($i:_) => () }

-/* error: invalid macro definition: missing fragment specifier */

+/* error: macro definition has parse errors */

macro_rules! m1 { ($$i) => () }

-/* error: invalid macro definition: `$$` is not allowed on the pattern side */

+/* error: macro definition has parse errors */

macro_rules! m2 { () => ( ${invalid()} ) }

-/* error: invalid macro definition: invalid metavariable expression */

+/* error: macro definition has parse errors */

"#]],

)

}

@@ -137,18 +137,18 @@ macro_rules! m9 { ($($($($i:ident)?)*)+) => {}; }

macro_rules! mA { ($($($($i:ident)+)?)*) => {}; }

macro_rules! mB { ($($($($i:ident)+)*)?) => {}; }

-/* error: invalid macro definition: empty token tree in repetition */

+/* error: macro definition has parse errors */

"#]],

);

}

diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs b/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs
index 6560d0ec46..bf70119838 100644
--- a/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mbe/metavar_expr.rs

@@ -275,9 +275,9 @@ macro_rules! depth_too_large {

}

fn test() {

- /* error: invalid macro definition: invalid metavariable expression */;

+ /* error: macro definition has parse errors */;

}

"#]],

);

diff --git a/crates/hir-def/src/macro_expansion_tests/mbe/tt_conversion.rs b/crates/hir-def/src/macro_expansion_tests/mbe/tt_conversion.rs
index ae56934f63..362c189f6a 100644
--- a/crates/hir-def/src/macro_expansion_tests/mbe/tt_conversion.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mbe/tt_conversion.rs

@@ -97,8 +97,8 @@ m2!(x

macro_rules! m1 { ($x:ident) => { ($x } }

macro_rules! m2 { ($x:ident) => {} }

-/* error: invalid macro definition: expected subtree */

-/* error: invalid token tree */

+/* error: macro definition has parse errors */

+/* error: mismatched delimiters */

"#]],

)

}

diff --git a/crates/hir-expand/src/builtin_fn_macro.rs b/crates/hir-expand/src/builtin_fn_macro.rs
index 6d3de0e55d..90cd3af757 100644
--- a/crates/hir-expand/src/builtin_fn_macro.rs
+++ b/crates/hir-expand/src/builtin_fn_macro.rs

@@ -446,7 +446,7 @@ fn compile_error_expand(

) -> ExpandResult<tt::Subtree> {

let err = match &*tt.token_trees {

[tt::TokenTree::Leaf(tt::Leaf::Literal(it))] => match unquote_str(it) {

- Some(unquoted) => ExpandError::other(unquoted),

+ Some(unquoted) => ExpandError::other(unquoted.into_boxed_str()),

None => ExpandError::other("`compile_error!` argument must be a string"),

_ => ExpandError::other("`compile_error!` argument must be a string"),

diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
index 6a288cf919..7b62eaa028 100644
--- a/crates/hir-expand/src/db.rs
+++ b/crates/hir-expand/src/db.rs

@@ -108,7 +108,7 @@ pub trait ExpandDatabase: SourceDatabase {

fn macro_arg(

&self,

id: MacroCallId,

- ) -> ValueResult<Option<(Arc<tt::Subtree>, SyntaxFixupUndoInfo)>, Arc<Box<[SyntaxError]>>>;

+ ) -> ValueResult<(Arc<tt::Subtree>, SyntaxFixupUndoInfo), Arc<Box<[SyntaxError]>>>;

/// Fetches the expander for this macro.

#[salsa::transparent]

#[salsa::invoke(TokenExpander::macro_expander)]

@@ -326,58 +326,77 @@ fn macro_arg(

db: &dyn ExpandDatabase,

id: MacroCallId,

// FIXME: consider the following by putting fixup info into eager call info args

- // ) -> ValueResult<Option<Arc<(tt::Subtree, SyntaxFixupUndoInfo)>>, Arc<Box<[SyntaxError]>>> {

-) -> ValueResult<Option<(Arc<tt::Subtree>, SyntaxFixupUndoInfo)>, Arc<Box<[SyntaxError]>>> {

- let mismatched_delimiters = |arg: &SyntaxNode| {

- let first = arg.first_child_or_token().map_or(T![.], |it| it.kind());

- let last = arg.last_child_or_token().map_or(T![.], |it| it.kind());

- let well_formed_tt =

- matches!((first, last), (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}']));

- if !well_formed_tt {

- // Don't expand malformed (unbalanced) macro invocations. This is

- // less than ideal, but trying to expand unbalanced macro calls

- // sometimes produces pathological, deeply nested code which breaks

- // all kinds of things.

- //

- // Some day, we'll have explicit recursion counters for all

- // recursive things, at which point this code might be removed.

- cov_mark::hit!(issue9358_bad_macro_stack_overflow);

- Some(Arc::new(Box::new([SyntaxError::new(

- "unbalanced token tree".to_owned(),

- arg.text_range(),

- )]) as Box<[_]>))

- } else {

- None

- }

- };

+ // ) -> ValueResult<Arc<(tt::Subtree, SyntaxFixupUndoInfo)>, Arc<Box<[SyntaxError]>>> {

+) -> ValueResult<(Arc<tt::Subtree>, SyntaxFixupUndoInfo), Arc<Box<[SyntaxError]>>> {

let loc = db.lookup_intern_macro_call(id);

if let Some(EagerCallInfo { arg, .. }) = matches!(loc.def.kind, MacroDefKind::BuiltInEager(..))

.then(|| loc.eager.as_deref())

.flatten()

{

- ValueResult::ok(Some((arg.clone(), SyntaxFixupUndoInfo::NONE)))

+ ValueResult::ok((arg.clone(), SyntaxFixupUndoInfo::NONE))

} else {

let (parse, map) = parse_with_map(db, loc.kind.file_id());

let root = parse.syntax_node();

let syntax = match loc.kind {

MacroCallKind::FnLike { ast_id, .. } => {

+ let dummy_tt = |kind| {

+ (

+ Arc::new(tt::Subtree {

+ delimiter: tt::Delimiter {

+ open: loc.call_site,

+ close: loc.call_site,

+ kind,

+ },

+ token_trees: Box::default(),

+ }),

+ SyntaxFixupUndoInfo::default(),

+ )

+ };

let node = &ast_id.to_ptr(db).to_node(&root);

let offset = node.syntax().text_range().start();

- match node.token_tree() {

- Some(tt) => {

- let tt = tt.syntax();

- if let Some(e) = mismatched_delimiters(tt) {

- return ValueResult::only_err(e);

- }

- tt.clone()

- }

- None => {

- return ValueResult::only_err(Arc::new(Box::new([

- SyntaxError::new_at_offset("missing token tree".to_owned(), offset),

- ])));

- }

+ let Some(tt) = node.token_tree() else {

+ return ValueResult::new(

+ dummy_tt(tt::DelimiterKind::Invisible),

+ Arc::new(Box::new([SyntaxError::new_at_offset(

+ "missing token tree".to_owned(),

+ offset,

+ )])),

+ );

+ };

+ let first = tt.left_delimiter_token().map(|it| it.kind()).unwrap_or(T!['(']);

+ let last = tt.right_delimiter_token().map(|it| it.kind()).unwrap_or(T![.]);

+ let mismatched_delimiters = !matches!(

+ (first, last),

+ (T!['('], T![')']) | (T!['['], T![']']) | (T!['{'], T!['}'])

+ );

+ if mismatched_delimiters {

+ // Don't expand malformed (unbalanced) macro invocations. This is

+ // less than ideal, but trying to expand unbalanced macro calls

+ // sometimes produces pathological, deeply nested code which breaks

+ // all kinds of things.

+ //

+ // So instead, we'll return an empty subtree here

+ cov_mark::hit!(issue9358_bad_macro_stack_overflow);

+ let kind = match first {

+ _ if loc.def.is_proc_macro() => tt::DelimiterKind::Invisible,

+ T!['('] => tt::DelimiterKind::Parenthesis,

+ T!['['] => tt::DelimiterKind::Bracket,

+ T!['{'] => tt::DelimiterKind::Brace,

+ _ => tt::DelimiterKind::Invisible,

+ };

+ return ValueResult::new(

+ dummy_tt(kind),

+ Arc::new(Box::new([SyntaxError::new_at_offset(

+ "mismatched delimiters".to_owned(),

+ offset,

+ )])),

+ );

}

+ tt.syntax().clone()

}

MacroCallKind::Derive { ast_id, .. } => {

ast_id.to_ptr(db).to_node(&root).syntax().clone()

@@ -427,15 +446,15 @@ fn macro_arg(

if matches!(loc.def.kind, MacroDefKind::BuiltInEager(..)) {

match parse.errors() {

- [] => ValueResult::ok(Some((Arc::new(tt), undo_info))),

+ [] => ValueResult::ok((Arc::new(tt), undo_info)),

errors => ValueResult::new(

- Some((Arc::new(tt), undo_info)),

+ (Arc::new(tt), undo_info),

// Box::<[_]>::from(res.errors()), not stable yet

Arc::new(errors.to_vec().into_boxed_slice()),

}

} else {

- ValueResult::ok(Some((Arc::new(tt), undo_info)))

+ ValueResult::ok((Arc::new(tt), undo_info))

}

@@ -519,21 +538,20 @@ fn macro_expand(

expander.expand(db, macro_call_id, &node, map.as_ref())

}

_ => {

- let ValueResult { value, err } = db.macro_arg(macro_call_id);

- let Some((macro_arg, undo_info)) = value else {

- return ExpandResult {

- value: CowArc::Owned(tt::Subtree {

- delimiter: tt::Delimiter::invisible_spanned(loc.call_site),

- token_trees: Box::new([]),

- }),

- // FIXME: We should make sure to enforce an invariant that invalid macro

- // calls do not reach this call path!

- err: Some(ExpandError::other("invalid token tree")),

- };

+ let ValueResult { value: (macro_arg, undo_info), err } = db.macro_arg(macro_call_id);

+ let format_parse_err = |err: Arc<Box<[SyntaxError]>>| {

+ let mut buf = String::new();

+ for err in &**err {

+ use std::fmt::Write;

+ _ = write!(buf, "{}, ", err);

+ }

+ buf.pop();

+ ExpandError::other(buf)

};

let arg = &*macro_arg;

- match loc.def.kind {

+ let res = match loc.def.kind {

MacroDefKind::Declarative(id) => {

db.decl_macro_expander(loc.def.krate, id).expand(db, arg.clone(), macro_call_id)

}

@@ -549,16 +567,7 @@ fn macro_expand(

MacroDefKind::BuiltInEager(..) if loc.eager.is_none() => {

return ExpandResult {

value: CowArc::Arc(macro_arg.clone()),

- err: err.map(|err| {

- let mut buf = String::new();

- for err in &**err {

- use std::fmt::Write;

- _ = write!(buf, "{}, ", err);

- }

- buf.pop();

- ExpandError::other(buf)

- }),

+ err: err.map(format_parse_err),

};

}

MacroDefKind::BuiltInEager(it, _) => {

@@ -570,6 +579,11 @@ fn macro_expand(

res

}

_ => unreachable!(),

+ };

+ ExpandResult {

+ value: res.value,

+ // if the arg had parse errors, show them instead of the expansion errors

+ err: err.map(format_parse_err).or(res.err),

}

};

@@ -597,17 +611,7 @@ fn macro_expand(

fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<Arc<tt::Subtree>> {

let loc = db.lookup_intern_macro_call(id);

- let Some((macro_arg, undo_info)) = db.macro_arg(id).value else {

- return ExpandResult {

- value: Arc::new(tt::Subtree {

- delimiter: tt::Delimiter::invisible_spanned(loc.call_site),

- token_trees: Box::new([]),

- }),

- // FIXME: We should make sure to enforce an invariant that invalid macro

- // calls do not reach this call path!

- err: Some(ExpandError::other("invalid token tree")),

- };

+ let (macro_arg, undo_info) = db.macro_arg(id).value;

let expander = match loc.def.kind {

MacroDefKind::ProcMacro(expander, ..) => expander,

diff --git a/crates/hir-expand/src/declarative.rs b/crates/hir-expand/src/declarative.rs
index 37084ee8b9..345e5cee26 100644
--- a/crates/hir-expand/src/declarative.rs
+++ b/crates/hir-expand/src/declarative.rs

@@ -44,9 +44,9 @@ impl DeclarativeMacroExpander {

)

});

match self.mac.err() {

- Some(e) => ExpandResult::new(

+ Some(_) => ExpandResult::new(

tt::Subtree::empty(tt::DelimSpan { open: loc.call_site, close: loc.call_site }),

- ExpandError::other(format!("invalid macro definition: {e}")),

+ ExpandError::MacroDefinition,

None => self

.mac

@@ -80,9 +80,9 @@ impl DeclarativeMacroExpander {

)

});

match self.mac.err() {

- Some(e) => ExpandResult::new(

+ Some(_) => ExpandResult::new(

tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),

- ExpandError::other(format!("invalid macro definition: {e}")),

+ ExpandError::MacroDefinition,

None => self.mac.expand(&tt, |_| (), new_meta_vars, call_site).map_err(Into::into),

}

diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index 42a8864c6a..3a7febc2eb 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs

@@ -44,7 +44,6 @@ use crate::{

builtin_derive_macro::BuiltinDeriveExpander,

builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander},

db::{ExpandDatabase, TokenExpander},

- fixup::SyntaxFixupUndoInfo,

hygiene::SyntaxContextData,

mod_path::ModPath,

proc_macro::{CustomProcMacroExpander, ProcMacroKind},

@@ -131,8 +130,9 @@ pub enum ExpandError {

UnresolvedProcMacro(CrateId),

/// The macro expansion is disabled.

MacroDisabled,

+ MacroDefinition,

Mbe(mbe::ExpandError),

- RecursionOverflowPoisoned,

+ RecursionOverflow,

Other(Box<Box<str>>),

ProcMacroPanic(Box<Box<str>>),

}

@@ -154,15 +154,14 @@ impl fmt::Display for ExpandError {

match self {

ExpandError::UnresolvedProcMacro(_) => f.write_str("unresolved proc-macro"),

ExpandError::Mbe(it) => it.fmt(f),

- ExpandError::RecursionOverflowPoisoned => {

- f.write_str("overflow expanding the original macro")

- }

+ ExpandError::RecursionOverflow => f.write_str("overflow expanding the original macro"),

ExpandError::ProcMacroPanic(it) => {

f.write_str("proc-macro panicked: ")?;

f.write_str(it)

}

ExpandError::Other(it) => f.write_str(it),

ExpandError::MacroDisabled => f.write_str("macro disabled"),

+ ExpandError::MacroDefinition => f.write_str("macro definition has parse errors"),

}

@@ -761,15 +760,7 @@ impl ExpansionInfo {

let (parse, exp_map) = db.parse_macro_expansion(macro_file).value;

let expanded = InMacroFile { file_id: macro_file, value: parse.syntax_node() };

- let (macro_arg, _) = db.macro_arg(macro_file.macro_call_id).value.unwrap_or_else(|| {

- (

- Arc::new(tt::Subtree {

- delimiter: tt::Delimiter::invisible_spanned(loc.call_site),

- token_trees: Box::new([]),

- }),

- SyntaxFixupUndoInfo::NONE,

- )

- });

+ let (macro_arg, _) = db.macro_arg(macro_file.macro_call_id).value;

let def = loc.def.ast_id().left().and_then(|id| {

let def_tt = match id.to_node(db) {

diff --git a/crates/ide-completion/src/tests/expression.rs b/crates/ide-completion/src/tests/expression.rs
index 78907a2896..e167f5c9ae 100644
--- a/crates/ide-completion/src/tests/expression.rs
+++ b/crates/ide-completion/src/tests/expression.rs

@@ -542,7 +542,26 @@ fn quux(x: i32) {

m!(x$0

}

"#,

- expect![[r#""#]],

+ expect![[r#"

+ fn quux(…) fn(i32)

+ lc x i32

+ lc y i32

+ ma m!(…) macro_rules! m

+ bt u32 u32

+ kw crate::

+ kw false

+ kw for

+ kw if

+ kw if let

+ kw loop

+ kw match

+ kw return

+ kw self::

+ kw true

+ kw unsafe

+ kw while

+ kw while let

+ "#]],

);

}

diff --git a/crates/ide-diagnostics/src/handlers/macro_error.rs b/crates/ide-diagnostics/src/handlers/macro_error.rs
index e4cb53f3a2..6a957ac1c9 100644
--- a/crates/ide-diagnostics/src/handlers/macro_error.rs
+++ b/crates/ide-diagnostics/src/handlers/macro_error.rs

@@ -242,7 +242,7 @@ macro_rules! foo {

fn f() {

foo!();

- //^^^ error: invalid macro definition: expected subtree

+ //^^^ error: macro definition has parse errors

}

"#,