Skip to content

Span::resolved_at and Span::located_at to combine behavior of two spans #47

New issue

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

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

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 6, 2018

Conversation

dtolnay
Copy link
Owner

@dtolnay dtolnay commented Jan 6, 2018

Added upstream in rust-lang/rust#47149.

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.

@dtolnay
Copy link
Owner Author

dtolnay commented Jan 6, 2018

Examples of use: heapsize_derive and lazy-static.

@mystor
Copy link
Contributor

mystor commented Jan 6, 2018

Nice! Those demos are pretty awesome looking :-)

@alexcrichton
Copy link
Contributor

Looks great! Should these be behind the line/column same feature gate? Or are these too important to pass up?

@dtolnay
Copy link
Owner Author

dtolnay commented Jan 6, 2018

I don't think these should be feature gated. Unlike Span::join and Span::source_file and the various custom error APIs, these are something I believe we need to build strong best practices around from the beginning. If these eventually change, I would see that as important enough to do a major version of proc-macro2 (again, unlike something like Span::source_file).

@dtolnay dtolnay merged commit 9025575 into master Jan 6, 2018
@dtolnay dtolnay deleted the combine branch January 6, 2018 17:19
@alexcrichton
Copy link
Contributor

Sounds good to me!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants