Use the call site for the assign suggestion on macro-expanded types#158509
Use the call site for the assign suggestion on macro-expanded types#158509ChronoCoders wants to merge 1 commit into
Conversation
When a `let` binding's type is actually a macro call such as `let x: vec![];`, lowering reports "expected type, found associated function call" and suggests writing `=` instead. The suggestion span was built from the expanded type span, which resolves inside the macro definition, so the suggestion pointed at the standard library source and proposed editing the expansion of `vec!`. Compute the span from `hir_ty.span.source_callsite()` so it maps back to the user's code. For non-macro types `source_callsite` returns the same span, so existing suggestions are unchanged.
|
HIR ty lowering was modified cc @fmease |
rustbot
added
S-waiting-on-review
|
r? @jieyouxu rustbot has assigned @jieyouxu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
A job failed! Check out the build log: (web) (plain enhanced) (plain) Click to see the possible cause of the failure (guessed by this bot) |
rustbot
removed
the
S-waiting-on-review
rustbot
added
the
S-waiting-on-review
|
r? me @bors r+ rollup |
rust-bors
Bot
added
the
S-waiting-on-bors
rust-bors
Bot
removed
the
S-waiting-on-review
|
hey! first of all - thank you for fixing this! secondly, i'd like to encourage you to not use llms to write descriptio for PRs, this change is self documented and simple enough to not provide any description at all, but for some reason it's a whole essay about this fix for no reason 🤷♂️, please use your own language (even if not perfect english) to write descriptions |
|
⌛ Testing commit 503344e with merge 78e36b7... Workflow: https://github.com/rust-lang/rust/actions/runs/28313019164 |
…ivooeo Use the call site for the assign suggestion on macro-expanded types Fixes #158492. When a `let` binding's type is a macro invocation, for example: fn main() { let x: vec![]; } the compiler emits "expected type, found associated function call" and suggests using `=` to assign. The suggestion span was derived from the expanded type span, which points inside the macro definition, so the help pointed at the standard library and proposed editing the expansion of `vec!`: help: use `=` if you meant to assign --> library/alloc/src/macros.rs:44:9 | 44 - $crate::vec::Vec::new() 44 + = This maps the suggestion span back to the call site with `source_callsite()`, so it now points at the user's code: help: use `=` if you meant to assign | LL - let x: vec![]; LL + let x = vec![]; For non-macro types `source_callsite()` returns the same span, so the existing cases (`Vec::new()`, `S::new(())`, `i32::from_be(num)`, and so on) are unchanged. The same fix is applied to the sibling primitive-type branch. A regression case is added to tests/ui/suggestions/let-binding-init-expr-as-ty.rs.
There was a problem hiding this comment.
You probably wanna use Span::find_ancestor_in_same_ctxt instead, Span::source_callsite is too drastic and insensitive to the span of the pattern.
Maybe just (pseudo) "pat.between(ty.in(pat) or bail out)" or more generally(?) (pseudo) "pat.between(ty.in(pat) if some) or (pat.in(ty) if some).between(ty) or bail out", I haven't tried out anything. If none of these work well (i.e., don't address the snippets I gave), there might be other methods.
@bors r-
There was a problem hiding this comment.
The suggestion is still incorrect for
macro_rules! make {
() => { let x: vec![]; };
}
fn main() {
make!();
}namely
error: expected type, found associated function call
--> u.rs:5:16
|
5 | let x: vec![];
| ^^^^^^
...
12 | make!();
| ------- in this macro invocation
|
= note: this error originates in the macro `vec` which comes from the expansion of the macro `make` (in Nightly builds, run with -Z macro-backtrace for more info)
help: use `=` if you meant to assign
|
5 - let x: vec![];
5 + let = : vec![];
|
There was a problem hiding this comment.
Moreover for
macro_rules! make {
($pat:pat, $ty:ty) => {
let $pat: $ty;
};
}
fn main() {
make!(_, vec![]);
}your branch now suggests
error: expected type, found associated function call
--> u.rs:8:14
|
8 | make!(_, vec![]);
| ^^^^^^
|
help: use `=` if you meant to assign
|
8 - make!(_, vec![]);
8 + make!(_ = vec![]);
|
main also emits an incorrect suggestion here but it's a different one.
There was a problem hiding this comment.
What's more, for the following input
macro_rules! make {
($pat:pat) => {
let $pat: Vec::new();
};
}
fn main() {
make!(_);
}main correctly emits
error: expected type, found associated function call
--> u.rs:3:19
|
3 | let $pat: Vec::new();
| ^^^^^^^^^^
...
8 | make!(_);
| -------- in this macro invocation
|
= note: this error originates in the macro `make` (in Nightly builds, run with -Z macro-backtrace for more info)
help: use `=` if you meant to assign
|
3 - let $pat: Vec::new();
3 + let $pat = Vec::new();
but your branch incorrectly emits
error: expected type, found associated function call
--> u.rs:3:19
|
3 | let $pat: Vec::new();
| ^^^^^^^^^^
...
8 | make!(_);
| -------- in this macro invocation
|
= note: this error originates in the macro `make` (in Nightly builds, run with -Z macro-backtrace for more info)
help: use `=` if you meant to assign
|
8 - make!(_);
8 + make!( = );
|
|
This pull request was unapproved. Auto build was cancelled due to unapproval. Cancelled workflows: |
rustbot
removed
the
S-waiting-on-author
|
Closing this. For the record: I'm a native English speaker and wrote that description myself. The comment "use your own language, even if not perfect English" is patronizing and based on an assumption with no evidence. A PR description being thorough is not proof of LLM use. I hope future contributors are treated with more good faith. |
|
Thanks, for explaining this.
Below is the reasons why I think so: Broken formatting (this is how your message looks when press "edit" so it's original look) 
Usually it should like this, where you specify code block using triple backtick 
Your description does not using any code blocks and using spacing that makes no sense to me 
Look once again how inconsistent and so much space loosing it is This is how copied text looks like after copying from somewhere (here's an real example (one of many and many such cases) with same broken formatting rust-lang/portable-simd#491)
Not sure where you saw there a bad faith. I was thankful and not judgeful at all. I just told you my feelings towards description because such bloated (in an era of LLM for a simple 2 line change having such big description from new contributor most of the times means it was LLM generated) description usually put an additional work to maintainers |
The description was prose, not code. Prose doesn't require triple backticks.
Most of the time ≠ this time. You assumed, you were wrong. That's the bad faith I mentioned — not your tone, your reasoning. Assuming incompetence or AI authorship from a new contributor because they wrote more than two lines is exactly the problem. |
|
Let's take another random PRs from first time contributor and what we will see there - #158473, #157690, #156993 The same problems with spacing, moreover I'm interested in why new contributors (and only) using such interesting wrapping style Interesting but your previous PR - #158376 have no problems with spacing, what's changed since then? |
6 participants
Fixes #158492.
When a
letbinding's type is a macro invocation, for example:the compiler emits "expected type, found associated function call" and
suggests using
=to assign. The suggestion span was derived from theexpanded type span, which points inside the macro definition, so the
help pointed at the standard library and proposed editing the
expansion of
vec!:This maps the suggestion span back to the call site with
source_callsite(), so it now points at the user's code:For non-macro types
source_callsite()returns the same span, so theexisting cases (
Vec::new(),S::new(()),i32::from_be(num), and soon) are unchanged. The same fix is applied to the sibling
primitive-type branch.
A regression case is added to
tests/ui/suggestions/let-binding-init-expr-as-ty.rs.