Macro diagnostics tweaks #55292
Macro diagnostics tweaks #55292
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| | | | ||
| | caused by the macro expansion here | ||
| | | ||
| = note: the usage of `foo!` is likely invalid in expression context |
There was a problem hiding this comment.
@dtolnay can you take a look and verify wether this suggestion is reasonable (it won't be wrong more often than not)?
There was a problem hiding this comment.
This is one case where it'd be the wrong approach:
fn main() {
macro_rules! a {
($e:expr) => { $e; 2}
}
a!(true);
}
error: macro expansion ignores token `2` and any following
--> file.rs:3:28
|
3 | ($e:expr) => { $e; 2}
| ^
4 | }
5 | a!(true)
| --------- help: you might be missing a semicolon here: `;`
| |
| caused by the macro expansion here
|
= note: the usage of `a!` is likely invalid in expression context
but the suggestion actually gets the code to compile...
There was a problem hiding this comment.
I think this suggestion will be helpful. 👍
| LL | let i = m!(); | ||
| | ^^^^ | ||
| | ----- help: you might be missing a semicolon here: `;` |
There was a problem hiding this comment.
Can we not peek ahead to the source text immediately following the span and see if there's a semicolon as the next character before making such an obviously inapplicable help message?
There was a problem hiding this comment.
We're actually checking that, which makes me intrigued at why this isn't being picked up. My guess is that if spans were completely hygienic this suggestion would appear on line 13, after typeof. This seems to be the only edge-case caught by our tests (before I added the linked check, many other incorrect suggestions were being emitted). Given that, would you mind if we merge as is and add extra logic in a later PR?
|
This seems fine. I would like an answer to my Q about peeking at the next character rather than blindly stating that a semicolon might be missing when it clearly is present in the source. |
pnkfelix
added
S-waiting-on-author
|
@bors r+ |
|
📌 Commit ad144ac has been approved by |
bors
added
S-waiting-on-bors
|
📌 Commit f8818cb has been approved by |
|
@bors rollup |
Macro diagnostics tweaks Fix rust-lang#30128, fix rust-lang#10951 by adding an appropriate span to the diagnostic. Fix rust-lang#26288 by suggesting adding semicolon to macro call.
Macro diagnostics tweaks Fix rust-lang#30128, fix rust-lang#10951 by adding an appropriate span to the diagnostic. Fix rust-lang#26288 by suggesting adding semicolon to macro call.
Rollup of 21 pull requests Successful merges: - #54816 (Don't try to promote already promoted out temporaries) - #54824 (Cleanup rustdoc tests with `@!has` and `@!matches`) - #54921 (Add line numbers option to rustdoc) - #55167 (Add a "cheap" mode for `compute_missing_ctors`.) - #55258 (Fix Rustdoc ICE when checking blanket impls) - #55264 (Compile the libstd we distribute with -Ccodegen-unit=1) - #55271 (Unimplement ExactSizeIterator for MIR traversing iterators) - #55292 (Macro diagnostics tweaks) - #55298 (Point at macro definition when no rules expect token) - #55301 (List allowed tokens after macro fragments) - #55302 (Extend the impl_stable_hash_for! macro for miri.) - #55325 (Fix link to macros chapter) - #55343 (rustbuild: fix remap-debuginfo when building a release) - #55346 (Shrink `Statement`.) - #55358 (Remove redundant clone (2)) - #55370 (Update mailmap for estebank) - #55375 (Typo fixes in configure_cmake comments) - #55378 (rustbuild: use configured linker to build boostrap) - #55379 (validity: assert that unions are non-empty) - #55383 (Use `SmallVec` for the queue in `coerce_unsized`.) - #55391 (bootstrap: clean up a few clippy findings)
Fix #30128, fix #10951 by adding an appropriate span to the diagnostic.
Fix #26288 by suggesting adding semicolon to macro call.