Span::resolved_at and Span::located_at to combine behavior of two spans #47149
Conversation
Proc macro spans serve two mostly unrelated purposes: controlling name resolution and controlling error messages. It can be useful to mix the name resolution behavior of one span with the line/column error message locations of a different span. In particular, consider the case of a trait brought into scope within the def_site of a custom derive. I want to invoke trait methods on the fields of the user's struct. If the field type does not implement the right trait, I want the error message to underline the corresponding struct field. Generating the method call with the def_site span is not ideal -- it compiles and runs but error messages sadly always point to the derive attribute like we saw with Macros 1.1. ``` | 4 | #[derive(HeapSize)] | ^^^^^^^^ ``` Generating the method call with the same span as the struct field's ident or type is not correct -- it shows the right underlines but fails to resolve to the trait in scope at the def_site. ``` | 7 | bad: std::thread::Thread, | ^^^^^^^^^^^^^^^^^^^^^^^^ ``` The correct span for the method call is one that combines the def_site's name resolution with the struct field's line/column. ``` field.span.resolved_at(Span::def_site()) // equivalently Span::def_site().located_at(field.span) ``` Adding both because which one is more natural will depend on context.
|
I don't understand how/when you're supposed to use these methods; the comments attached to them aren't very helpful. In particular, what do you mean by (emphasis mine):
What is failing to resolve which trait where, why should it, and why does it matter? Perhaps you could add some ui-tests that clearly show what these methods let you do that you couldn't do before? |
|
@SergioBenitez check out #45934 for some background. The span of the tokens in a method call or type name or basically any other
|
|
Regarding documentation of call_site vs def_site and other things one would need to know about hygiene in proc macros, I believe that is part of what is being tracked in #38356 and will be a blocker for stabilization. |
|
While I'm aware of how
I now understand the issue. In effect, you're trying to emit names that you want Rust to resolve as if they were from the definition site. In particular, a method call that requires a trait to be in scope. Why can't you generate a Also, I'm curious about this:
Shouldn't the fields have the same name resolution as |
There is already a
The fields in the input to a custom derive are contained within the input to the custom derive and so they come from the call site. This is why in |
kennytm
added
the
S-waiting-on-review
|
Discussed in IRC -- I understand Sergio's comments and concerns better after confirming that he had his |
|
Yes, my apologies. I was mixing up I was also under the impression that let trait_call = quote_span!(field.span, #field.method_call());
return quote!(#trait_call);Thus, Instead, however, // `method_call()` incorrectly resolves at call site, correctly has field span position
let trait_call = quote_span!(field.span, #field.method_call());
return quote!(#trait_call);
// `method_call()` correctly resolves at def site, incorrectly has def site span position
return quote!(#field.method_call());Is there a reason that |
|
@bors r+ |
|
📌 Commit 000e907 has been approved by |
Yeah, if the interpolated tokens come from the macro invocation, you typically don't want to change how they resolve. E.g. say I have a macro that passes its input to a function: #[proc_macro]
fn m(input: TokenStream) -> TokenStream {
quote! {
extern crate my_crate;
my_crate::my_function($input);
}
}Here, I want |
Span::resolved_at and Span::located_at to combine behavior of two spans Proc macro spans serve two mostly unrelated purposes: controlling name resolution and controlling error messages. It can be useful to mix the name resolution behavior of one span with the line/column error message locations of a different span. In particular, consider the case of a trait brought into scope within the def_site of a custom derive. I want to invoke trait methods on the fields of the user's struct. If the field type does not implement the right trait, I want the error message to underline the corresponding struct field. Generating the method call with the def_site span is not ideal -- it compiles and runs but error messages sadly always point to the derive attribute like we saw with Macros 1.1. ``` | 4 | #[derive(HeapSize)] | ^^^^^^^^ ``` Generating the method call with the same span as the struct field's ident or type is not correct -- it shows the right underlines but fails to resolve to the trait in scope at the def_site. ``` | 7 | bad: std::thread::Thread, | ^^^^^^^^^^^^^^^^^^^^^^^^ ``` The correct span for the method call is one that combines the def_site's name resolution with the struct field's line/column. ```rust field.span.resolved_at(Span::def_site()) // equivalently Span::def_site().located_at(field.span) ``` Adding both because which one is more natural will depend on context. Addresses rust-lang#38356 (comment). r? @jseyfried
Span::resolved_at and Span::located_at to combine behavior of two spans Proc macro spans serve two mostly unrelated purposes: controlling name resolution and controlling error messages. It can be useful to mix the name resolution behavior of one span with the line/column error message locations of a different span. In particular, consider the case of a trait brought into scope within the def_site of a custom derive. I want to invoke trait methods on the fields of the user's struct. If the field type does not implement the right trait, I want the error message to underline the corresponding struct field. Generating the method call with the def_site span is not ideal -- it compiles and runs but error messages sadly always point to the derive attribute like we saw with Macros 1.1. ``` | 4 | #[derive(HeapSize)] | ^^^^^^^^ ``` Generating the method call with the same span as the struct field's ident or type is not correct -- it shows the right underlines but fails to resolve to the trait in scope at the def_site. ``` | 7 | bad: std::thread::Thread, | ^^^^^^^^^^^^^^^^^^^^^^^^ ``` The correct span for the method call is one that combines the def_site's name resolution with the struct field's line/column. ```rust field.span.resolved_at(Span::def_site()) // equivalently Span::def_site().located_at(field.span) ``` Adding both because which one is more natural will depend on context. Addresses rust-lang#38356 (comment). r? @jseyfried
proc_macro: Stabilize `Span::resolved_at` and `Span::located_at` Introduced in rust-lang#47149. Part of rust-lang#54725. Motivation: rust-lang#68716 (comment). Identifiers in proc macros may want to inherit span locations for diagnostics from one tokens (e.g. some tokens from the macro input), but resolve those identifiers from some different location (e.g. from the macro's definition site). This becomes especially important when multiple resolution locations become available with stabilization of [`Span::mixed_site`](rust-lang#68716). Why I think this is the right API for setting span's location and hygiene - rust-lang#69041 (comment). r? @dtolnay
Proc macro spans serve two mostly unrelated purposes: controlling name resolution and controlling error messages. It can be useful to mix the name resolution behavior of one span with the line/column error message locations of a different span.
In particular, consider the case of a trait brought into scope within the def_site of a custom derive. I want to invoke trait methods on the fields of the user's struct. If the field type does not implement the right trait, I want the error message to underline the corresponding struct field.
Generating the method call with the def_site span is not ideal -- it compiles and runs but error messages sadly always point to the derive attribute like we saw with Macros 1.1.
Generating the method call with the same span as the struct field's ident or type is not correct -- it shows the right underlines but fails to resolve to the trait in scope at the def_site.
The correct span for the method call is one that combines the def_site's name resolution with the struct field's line/column.
Adding both because which one is more natural will depend on context.
Addresses #38356 (comment). r? @jseyfried