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.

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

Compiler suggests #[derive((DeriveMacro).try_into().unwrap())] #78862

Open
msrd0 opened this issue Nov 7, 2020 · 7 comments
Open

Compiler suggests #[derive((DeriveMacro).try_into().unwrap())] #78862

msrd0 opened this issue Nov 7, 2020 · 7 comments
Labels
A-proc-macros Area: Procedural macros A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` 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

@msrd0
Copy link
Contributor

msrd0 commented Nov 7, 2020

Today I encountered a rather unhelpful suggestion from the compiler (1.47.0):

error[E0308]: mismatched types                                                                                                                                                                                                                                               
 --> src/lib.rs:3:10                                                                                                                                                                                                                              
  |                                                                                                                                                                                                                                                                          
3 | #[derive(DeriveMacro)]                                                                                                                                                                                                                                                      
  |          ^^^^^^^^^^^ expected `usize`, found `i32`                                                                                                                                                                                                                          
  |                                                                                                                                                                                                                                                                          
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)                                                                                                                                                             
help: you can convert an `i32` to `usize` and panic if the converted value wouldn't fit                                                                                                                                                                                      
  |                                                                                                                                                                                                                                                                          
3 | #[derive((DeriveMacro).try_into().unwrap())]                                                                                                                                                                                                                                
  |   

The code emited from the derive macro contains something along the lines of let value: usize = -1 as i32.

Applying the suggestion, however, doesn't really help (almost as if I had to fix the problem in the derive macro):

error: malformed `derive` attribute input
 --> src/lib.rs:3:1
  |
3 | #[derive((DeriveMacro).try_into().unwrap())]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: missing traits to be derived: `#[derive(Trait1, Trait2, ...)]`

I believe the compiler should only emit that suggestion if the code it applies to is actually an expression of type i32.

@Aaron1011
Copy link
Member

@msrd0 In general, there's not much that the compiler can do in this type of situation - it's up to the proc-macro to emit tokens with reasonable spans.

Is DeriveMacro the actual name of the macro you're invoking? Are you able to share your repository, or a link to the crate that defines the macro you're invoking?

@msrd0
Copy link
Contributor Author

msrd0 commented Nov 8, 2020

This is a small extract of my macro that reproduces the error and suggestion: https://github.com/msrd0/proc-macro-try-into-suggestion

I believe the let value: usize part should have a span of the macro's call site (#[derive(DeriveMacro)]) while -1 as i32 should point into the non-derive code (#[custom = "{ -1 as i32 }"]), not sure which span is important here.

So maybe this would've been a better suggestion, eventhough the code will always panic:

help: you can convert an `i32` to `usize` and panic if the converted value wouldn't fit                                                                                                                                                                                      
  |                                                                                                                                                                                                                                                                          
4 | #[custom = "{ (-1 as i32).try_into().unwrap() }"]                                                                                                                                                                                                                                
  |   

@msrd0
Copy link
Contributor Author

msrd0 commented Nov 8, 2020

Still, the error message is unhelpful even if the code was completely unrelated to the macro's input and hence cannot point to a reasonable span outside of the generated code. So maybe, if the only available span is that of the call site, do not emit any suggestion?

@jyn514 jyn514 added A-proc-macros Area: Procedural macros A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` 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. labels Nov 8, 2020
@msrd0
Copy link
Contributor Author

msrd0 commented Nov 9, 2020

Testing this further, it seems that the compiler is confused by the span of every token in -1 as i32 pointing to the string literal "-1 as i32". If I used my own variation of syn::Meta that allows syn::Expr instead of syn::Lit, I was able to get #[custom = (-1 as i32).try_into().unwrap())] for input #[custom = -1 as i32] as the suggestion, which is actually what I'd expect.

@danielhenrymantilla
Copy link
Contributor

danielhenrymantilla commented Nov 9, 2020

Yes, I don't think spans can point inside a string literal, at least not in stable Rust. The best you could achieve is to manually respan the parsed TokenStream from the LitStr with the span of that very LitStr (otherwise it defaults to the call_site() span), but then it would suggest doing: #[custom = "-1 as i32".try_into().unwrap()]. So, if you care about the suggestion being on-point, and on stable Rust, you should indeed use a syntax that does not take a string literal but a code block, something like: #[custom = { -1 as i32 }] or even #[custom = || { -1 as i32 }] so that the "non evaluated" nature is more explicit.


w.r.t. the compiler, maybe some mechanism to express there is no actual span to apply suggestions to would be useful, so that you could use a LitStr (with an overridden span) and expect a message along the lines of:

error[E0308]: mismatched types                                                                                                                                                                                                                                               
 --> src/lib.rs:3:10                                                                                                                                                                                                                              
  |                                                                                                                                                                                                                                                                          
3 | #[derive(CustomMacro)]
4 | #[custom = "-1 as i32"]
  |            ^^^^^^^^^^^ expected `usize`, found `i32`                                                                                                                                                                                                                          
  |                                                                                                                                                                                                                                                                          
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) 

@msrd0
Copy link
Contributor Author

msrd0 commented Nov 9, 2020

The best you could achieve is to manually respan the parsed TokenStream from the LitStr with the span of that very LitStr

I didn't know that syn already supported this, but quote_spanned!(expr_span => #expr_parsed) should do exactly the same. Still, the compiler applies the suggestion to the call site and not the str.

w.r.t. the compiler, maybe some mechanism to express there is no actual span to apply suggestions to

This is probably the behaviour I expected to begin with when the span is that of the call site.

@phansch
Copy link
Member

phansch commented Jan 12, 2021

fwiw, I believe we're running into similar issues with various Clippy lints:

AIUI, there is currently no way to tell if a span originated from quote_spanned usage, right?

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 16, 2023
Do not provide suggestions when the spans come from expanded code that doesn't point at user code

Hide invalid proc-macro suggestions and track spans
coming from proc-macros pointing at attribute.

Effectively, unless the proc-macro keeps user spans,
suggestions will not be produced for the code they
produce.

r? `@ghost`

Fix rust-lang#107113, fix rust-lang#107976, fix rust-lang#107977, fix rust-lang#108748, fix rust-lang#106720, fix rust-lang#90557.

Could potentially address rust-lang#50141, rust-lang#67373, rust-lang#55146, rust-lang#78862, rust-lang#74043, rust-lang#88514, rust-lang#83320, rust-lang#91520, rust-lang#104071. CC rust-lang#50122, rust-lang#76360.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2023
Do not provide suggestions when the spans come from expanded code that doesn't point at user code

Hide invalid proc-macro suggestions and track spans
coming from proc-macros pointing at attribute.

Effectively, unless the proc-macro keeps user spans,
suggestions will not be produced for the code they
produce.

r? ``@ghost``

Fix rust-lang#107113, fix rust-lang#107976, fix rust-lang#107977, fix rust-lang#108748, fix rust-lang#106720, fix rust-lang#90557.

Could potentially address rust-lang#50141, rust-lang#67373, rust-lang#55146, rust-lang#78862, rust-lang#74043, rust-lang#88514, rust-lang#83320, rust-lang#91520, rust-lang#104071. CC rust-lang#50122, rust-lang#76360.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-proc-macros Area: Procedural macros A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` 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
Development

No branches or pull requests

5 participants