`Semantics::expand_macro_call` returns `None` on some macro calls from version 0.0.274 onwards

[redsun82]

In our experimental CodeQL rust analysis we piggy-back on rust-analyzer analysis library, and specifically for macro calls we rely on ra_ap_hir::Semantics::expand_macro_call. While updating from rust-analyzer 0.0.273 to 0.0.274 we suddenly saw in the wild a significant number of cases of that function returning None. I managed to recreate a small reproduction crate of the 4 classes of unexpanded macros I saw so far. For 2 of them I also managed to reproduce the behaviour on the VSCode extension (that is, newer versions give no expansion with the Expand macro recursively at caret command, while an older one those. The crate can be accessed here. Here's the 4 cases I found:

1. a macro call in an impl block expanding to some items:

   impl S {
       def_x!();  // this doesn't expand since 0.0.274
   }

   (this does still expand in VSCode)
2. very specifically, a macro call providing a format specifier for a macro that expands to a format! call (the original example was anyhow!, I recreated it with

   macro_rules! my_macro {
       ($head:expr, $($tail:tt)*) => { format!($head, $($tail)*) };
   }
   
   
   fn test() {
       _ = my_macro!(
           concat!("<", "{}", ">"),  // this doesn't expand since 0.0.274
           "hi",
       );
   }

   (VSCode does not expand further than alloc::__export::format_args! either before or after, but I'm guessing that might be on purpose as format_args! is not really a macro. Still I was getting an expansion of concat! there in 0.0.273, but I'm getting none on 0.0.274)
3. a macro call in a doc attribute like #[doc = include_str!("README.md")]. This one I can confirm that previous versions of VSCode were able to expand, but now I get no expansion.
4. a macro call in a .rs file that is included via include! rather than via a normal module item (and apparently that has been done in the wild even for non-generated files). Again when going to the included file with VSCode, the macros can be expanded on older plugin versions, but can't any more on newer ones.

I've seen some substantial changes in that area in the code, but I can't put my finger on what could be causing this. For us the most concerning instance is the first one, as it's not that uncommon.

Do you have any guidance on this?

rust-analyzer version: 0.0.274 (rust-analyzer as library from crates.io)
rustc version: 1.86

editor or extension: -

relevant settings: -

repository link (if public, optional): example crate

[ChayimFriedman2]

a macro call in an impl block expanding to some items:

Funnily enough, this was fixed just 18 hours ago (#19862). Check again with latest nightly. Also check the other cases, just to be sure.

[Veykril]

Are you sure #19862 fixed that? The issue here involves assoc item macro calls, which are special on their own.

[ChayimFriedman2]

Oh right assoc items have their own handling of macros...

I think we should reverse the code back to relowering the macro. It is simpler and maybe will also allow us to avoid putting the macro calls in a side table.

[Veykril]

I disagree, the relowering is quite error prone and weird. It is also annoying code duplication that needs special considerations.

[ChayimFriedman2]

Well, that means

• Traversing the ancestor trait/impl (and could it be outside macro calls),
• Recording eager macros in the def map/source map, because we don't currently AFAIK,
• and at the end, still lower for some macros, because we don't expand macros in docstrings at all.

[Veykril]

Well, that means

* Traversing the ancestor trait/impl (and could it be outside macro calls),

I am not sure I understand this part, looking at the code we just do not record the macro calls in semantics.
Edit: Ah, this goes through a different code path ...

* Recording eager macros in the def map/source map, because we don't currently AFAIK,

We do, I believe I fixed that recently, so that probably just lacks some recording in the semantics layer.

* and at the end, still lower for some macros, because we don't expand macros in docstrings at all.

Well, we do not support macro calls in attribute position at all, so that's not really relevant imo. This is generally an unsolved problem still for us. I am more surprised we managed to do that in the ide layer at all before.

[Veykril]

#19879 fixes #1 and #3. #2 needs a proper implementation and #4 should work but it doesn't (not sure yet why).

[redsun82]

That's great news, thanks a lot! I think we can work around n4, after all we do a pass on the included code in the context of the one or more sources that include that, so we can work with that data (we should probably just skip looking at those included files directly). n2 is a quite specific case, the only instance in the wild I've seen was an anyhow!(concat!(...), ...), we can live with that. n1 was the one we were most worried about, so many thanks for fixing that!

[Veykril]

Oh sorry, I mixed the numbneres up. #2 should work now, #3 is not supported

[redsun82]

ah, that's even better! I don't think n3 is that relevant for code analysis, so I think we're fine with that not working.

(also, I edited the issue to use a numbered list instead of bullet points, will probably be clearer 🙂)