rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.2k stars 1.6k forks source link

Expanding a macro which does not compile produces unexpected results. #13833

Open tfpk opened 1 year ago

tfpk commented 1 year ago

rust-analyzer version: rust-analyzer version: 0.0.0 (d2281f036 2022-11-25) and rust-analyzer version: 0.3.1325-standalone

rustc version: rustc 1.66.0 (69f9c33d7 2022-12-12)

relevant settings: N/A

In the following situation:

macro_rules! foo {
    (foo) => (true);
    () => (false);
}
fn f() { let result = foo$0!(asdfasdf); }

When running Expand Macro Recursively, the result is:

// Recursive expansion of foo! macro
// ==================================

true

When the correct result should be a failure to expand on the basis that:

error: no rules expected the token `asdfasdf`
 --> src/main.rs:5:28
  |
1 | macro_rules! foo {
  | ---------------- when calling this macro
...
5 | fn f() { let result = foo!(asdfasdf); }
  |                            ^^^^^^^^ no rules expected this token in macro call
flodiebold commented 1 year ago

We do actually report the error, if you have enabled the experimental macro-error diagnostic. You get a result anyway because our macro expansion needs to be resilient to errors to make e.g. completion in macros work (and, for example with this macro, it's nicer to report that result has type bool instead of saying its type can't be inferred). I'm not sure refusing to expand the macro would be an improvement.

We could add a note saying "there were errors during macro expansion" though :thinking:

tfpk commented 1 year ago

Thanks for the explanation -- that makes a lot of sense.

I think what would have helped me would have been a comment somewhere (in expand_macro.rs? or maybe in hir/src/source_analyzer.rs? ) that explains this rationale. I came across this "bug" when working on #13810 , and I spent a while trying to figure out what I'd done to "break" macro expansion, so this was just very confusing.