Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Suggestions are often invalid for proc-macro produced code #85932

Closed
m-ou-se opened this issue Jun 2, 2021 · 3 comments · Fixed by #85937
Closed

Suggestions are often invalid for proc-macro produced code #85932

m-ou-se opened this issue Jun 2, 2021 · 3 comments · Fixed by #85937
Assignees
@m-ou-se
Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Jun 2, 2021

The tokens generated by proc_macro (e.g. from quote!()) use Span::call_site(), which makes some suggestions produce invalid output:

error[E0308]: mismatched types
 --> src/main.rs:3:1
  |
3 | #[hello]
  | ^^^^^^^^
  | |
  | expected `&mut i32`, found integer
  | help: consider mutably borrowing here: `&mut #[hello]`

It's a bit tricky, because this call_site span does point into the current crate (to #[hello]), so this situaiton isn't detected in the same way as external macro_rules.
@m-ou-se m-ou-se added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Jun 2, 2021
@m-ou-se m-ou-se self-assigned this Jun 2, 2021
@m-ou-se
Copy link
Member Author

m-ou-se commented Jun 2, 2021

Looks like rustc_middle::lint::in_external_macro should be used in more places. That one detects this call_site span by checking if def_site is a dummy or imported.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jun 2, 2021

I'm working on a fix specifically for these typeck/borrowing suggestions. Not sure how many other suggestions have the same problem.

@m-ou-se m-ou-se mentioned this issue Jun 2, 2021
Merged
@estebank
Copy link
Contributor

estebank commented Jun 3, 2021

@m-ou-se this is a prevalent problem. We need to add metadata to spans to distinguish call sites explicitly in an easy way and always ignore suggestions in them.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 3, 2021
@JohnTitor
Rollup merge of rust-lang#85937 - m-ou-se:macro-ref-suggestions, r=es…
2bd3237 
…tebank

Fix bad suggestions for code from proc_macro

Fixes rust-lang#85932

This disables these suggestions for spans from external macros, while keeping them for macros defined locally:

Before:
```
3 | #[hello]
  | ^^^^^^^^
  | |
  | expected `&mut i32`, found integer
  | help: consider mutably borrowing here: `&mut #[hello]`
```

After:
```
3 | #[hello]
  | ^^^^^^^^ expected `&mut i32`, found integer
```

Unchanged:
```
26 | macro_rules! bla { () => { x(123); } }
   |                              ^^^
   |                              |
   |                              expected `&mut i32`, found integer
   |                              help: consider mutably borrowing here: `&mut 123`
...
29 |     bla!();
   |     ------- in this macro invocation
```
@bors bors closed this as completed in 99fc56b Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@m-ou-se m-ou-se

Labels
A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix`. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix bad suggestions for code from proc_macro m-ou-se/rust
2 participants
@m-ou-se @estebank