Avoid empty extra-arg suggestion spans in macro calls#152634
Avoid empty extra-arg suggestion spans in macro calls#152634TaKO8Ki wants to merge 2 commits intorust-lang:mainfrom
Conversation
|
r? @BoxyUwU rustbot has assigned @BoxyUwU. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? compiler |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
This is not an MCVE, it contains a lot of distracting elements and leads to distracting errors. Could you please use a more distilled version of the reproducer like:
#![feature(generic_assert)]
fn func(value: usize) {
assert!(core::mem::size_of(value, 1) >= 1);
}which only generates 1 error instead of 5.
There was a problem hiding this comment.
However note that this reproducer still relies on implementation details of generic_assert. Ideally, we'd define our own macro to be used in this test file. This way, if the implementation of generic_assert changes, this test won't accidentally & likely silently stop testing the right thing...
| help: remove the extra argument | ||
| | | ||
| LL - assert!(core::mem::size_of::<T>(val, 1) >= 1); | ||
| LL + assert!(core::mem::size_of::<T>(val, ) >= 1); |
There was a problem hiding this comment.
The new suggestion isn't quite right though. It should either suggest removing both extra arguments or if that's not possible due to differing syntax contexts we should err on the safe side and just not suggest anything (unless you can make it work).
The diagnostic knows that 2 args are extra but the suggestion no longer does for some reason.
There was a problem hiding this comment.
What happens if the source code was assert!(core::mem::size_of(value,)); does rustc now suggest assert!(core::mem::size_of(,)); under your PR? I don't actually know.
Fixes #152414
Fixes #131762
When building
Error::Extraremoval suggestions, we extended spans withtoanduntileven when syntax contexts differed inlabels_and_suggestion_text. In macro-expanded calls this could fall back to an empty span and triggering an ICE.I fixed it to combine spans only when contexts match.