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

Implement span quoting for proc-macros #84278

Merged
merged 2 commits into from
May 12, 2021

Conversation

Aaron1011
Copy link
Member

This PR implements span quoting, allowing proc-macros to produce spans
pointing into their own crate. This is used by the unstable
proc_macro::quote! macro, allowing us to get error messages like this:

error[E0412]: cannot find type `MissingType` in this scope
  --> $DIR/auxiliary/span-from-proc-macro.rs:37:20
   |
LL | pub fn error_from_attribute(_args: TokenStream, _input: TokenStream) -> TokenStream {
   | ----------------------------------------------------------------------------------- in this expansion of procedural macro `#[error_from_attribute]`
...
LL |             field: MissingType
   |                    ^^^^^^^^^^^ not found in this scope
   |
  ::: $DIR/span-from-proc-macro.rs:8:1
   |
LL | #[error_from_attribute]
   | ----------------------- in this macro invocation

Here, MissingType occurs inside the implementation of the proc-macro
#[error_from_attribute]. Previosuly, this would always result in a
span pointing at #[error_from_attribute]

This will make many proc-macro-related error message much more useful -
when a proc-macro generates code containing an error, users will get an
error message pointing directly at that code (within the macro
definition), instead of always getting a span pointing at the macro
invocation site.

This is implemented as follows:

  • When a proc-macro crate is being compiled, it causes the quote!
    macro to get run. This saves all of the sapns in the input to quote!
    into the metadata of the proc-macro-crate (which we are currently
    compiling). The quote! macro then expands to a call to
    proc_macro::Span::recover_proc_macro_span(id), where id is an
    opaque identifier for the span in the crate metadata.
  • When the same proc-macro crate is run (e.g. it is loaded from disk
    and invoked by some consumer crate), the call to
    proc_macro::Span::recover_proc_macro_span causes us to load the span
    from the proc-macro crate's metadata. The proc-macro then produces a
    TokenStream containing a Span pointing into the proc-macro crate
    itself.

The recursive nature of 'quote!' can be difficult to understand at
first. The file src/test/ui/proc-macro/quote-debug.stdout shows
the output of the quote! macro, which should make this eaier to
understand.

This PR also supports custom quoting spans in custom quote macros (e.g.
the quote crate). All span quoting goes through the
proc_macro::quote_span method, which can be called by a custom quote
macro to perform span quoting. An example of this usage is provided in
src/test/ui/proc-macro/auxiliary/custom-quote.rs

Custom quoting currently has a few limitations:

In order to quote a span, we need to generate a call to
proc_macro::Span::recover_proc_macro_span. However, proc-macros
support renaming the proc_macro crate, so we can't simply hardcode
this path. Previously, the quote_span method used the path
crate::Span - however, this only works when it is called by the
builtin quote! macro in the same crate. To support being called from
arbitrary crates, we need access to the name of the proc_macro crate
to generate a path. This PR adds an additional argument to quote_span
to specify the name of the proc_macro crate. Howver, this feels kind
of hacky, and we may want to change this before stabilizing anything
quote-related.

Additionally, using quote_span currently requires enabling the
proc_macro_internals feature. The builtin quote! macro
has an #[allow_internal_unstable] attribute, but this won't work for
custom quote implementations. This will likely require some additional
tricks to apply allow_internal_unstable to the span of
proc_macro::Span::recover_proc_macro_span.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 17, 2021
@Aaron1011
Copy link
Member Author

cc @dtolnay - from what I can tell, the quote crate is implemented via macro_rules! macros, not via a proc-macro. Do you think it would be feasible to add (opt-in) support for span quoting to the quote crate?

@rust-log-analyzer

This comment has been minimized.

@@ -135,6 +136,7 @@ pub fn quote(stream: TokenStream) -> TokenStream {
/// Quote a `Span` into a `TokenStream`.
/// This is needed to implement a custom quoter.
#[unstable(feature = "proc_macro_quote", issue = "54722")]
pub fn quote_span(_: Span) -> TokenStream {
quote!(crate::Span::def_site())
pub fn quote_span(proc_macro_crate: TokenStream, span: Span) -> TokenStream {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this is unstable, how will this change affect the ecosystem? I'm guessing some nightly only crates will stop working until they update their deps, right?

Copy link
Member Author

@Aaron1011 Aaron1011 Apr 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A search of Github didn't turn up any results (other than library/proc_macro/src/quote.rs in forks of rustc): https://github.com/search?l=Rust&p=1&q=quote_span&type=Code

I would be very surprised if anyone was using this:

  1. It currently doesn't do anything, so it's easier to manually use Span::call_site() or Span::def_site()
  2. This is only useful if you're writing your own custom quote macro - as far as I know, the quote crate is the only custom macro there is, and it doesn't use quote_span.

@crlf0710
Copy link
Member

crlf0710 commented May 8, 2021

Triage: CI is still red here.

@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-proc-macros Area: Procedural macros labels May 8, 2021
@petrochenkov petrochenkov self-assigned this May 8, 2021
@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2021
@Aaron1011 Aaron1011 force-pushed the feature/new-proc-macro-meta-span branch from ecd65e5 to a714345 Compare May 8, 2021 23:47
@rust-log-analyzer

This comment has been minimized.

@Aaron1011 Aaron1011 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 9, 2021
@estebank
Copy link
Contributor

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented May 11, 2021

📌 Commit 74e6edf8f6469664470e598c3bfd88342954b4e7 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2021
@bors
Copy link
Contributor

bors commented May 11, 2021

⌛ Testing commit 74e6edf8f6469664470e598c3bfd88342954b4e7 with merge ee28776c5a047da768ba6f418ba0317b6b868567...

@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 11, 2021
This PR implements span quoting, allowing proc-macros to produce spans
pointing *into their own crate*. This is used by the unstable
`proc_macro::quote!` macro, allowing us to get error messages like this:

```
error[E0412]: cannot find type `MissingType` in this scope
  --> $DIR/auxiliary/span-from-proc-macro.rs:37:20
   |
LL | pub fn error_from_attribute(_args: TokenStream, _input: TokenStream) -> TokenStream {
   | ----------------------------------------------------------------------------------- in this expansion of procedural macro `#[error_from_attribute]`
...
LL |             field: MissingType
   |                    ^^^^^^^^^^^ not found in this scope
   |
  ::: $DIR/span-from-proc-macro.rs:8:1
   |
LL | #[error_from_attribute]
   | ----------------------- in this macro invocation
```

Here, `MissingType` occurs inside the implementation of the proc-macro
`#[error_from_attribute]`. Previosuly, this would always result in a
span pointing at `#[error_from_attribute]`

This will make many proc-macro-related error message much more useful -
when a proc-macro generates code containing an error, users will get an
error message pointing directly at that code (within the macro
definition), instead of always getting a span pointing at the macro
invocation site.

This is implemented as follows:
* When a proc-macro crate is being *compiled*, it causes the `quote!`
  macro to get run. This saves all of the sapns in the input to `quote!`
  into the metadata of *the proc-macro-crate* (which we are currently
  compiling). The `quote!` macro then expands to a call to
  `proc_macro::Span::recover_proc_macro_span(id)`, where `id` is an
opaque identifier for the span in the crate metadata.
* When the same proc-macro crate is *run* (e.g. it is loaded from disk
  and invoked by some consumer crate), the call to
`proc_macro::Span::recover_proc_macro_span` causes us to load the span
from the proc-macro crate's metadata. The proc-macro then produces a
`TokenStream` containing a `Span` pointing into the proc-macro crate
itself.

The recursive nature of 'quote!' can be difficult to understand at
first. The file `src/test/ui/proc-macro/quote-debug.stdout` shows
the output of the `quote!` macro, which should make this eaier to
understand.

This PR also supports custom quoting spans in custom quote macros (e.g.
the `quote` crate). All span quoting goes through the
`proc_macro::quote_span` method, which can be called by a custom quote
macro to perform span quoting. An example of this usage is provided in
`src/test/ui/proc-macro/auxiliary/custom-quote.rs`

Custom quoting currently has a few limitations:

In order to quote a span, we need to generate a call to
`proc_macro::Span::recover_proc_macro_span`. However, proc-macros
support renaming the `proc_macro` crate, so we can't simply hardcode
this path. Previously, the `quote_span` method used the path
`crate::Span` - however, this only works when it is called by the
builtin `quote!` macro in the same crate. To support being called from
arbitrary crates, we need access to the name of the `proc_macro` crate
to generate a path. This PR adds an additional argument to `quote_span`
to specify the name of the `proc_macro` crate. Howver, this feels kind
of hacky, and we may want to change this before stabilizing anything
quote-related.

Additionally, using `quote_span` currently requires enabling the
`proc_macro_internals` feature. The builtin `quote!` macro
has an `#[allow_internal_unstable]` attribute, but this won't work for
custom quote implementations. This will likely require some additional
tricks to apply `allow_internal_unstable` to the span of
`proc_macro::Span::recover_proc_macro_span`.
The spans generated by `quote!` are (intentionally) no longer all the
same, so I removed that check entirely.
@Aaron1011 Aaron1011 force-pushed the feature/new-proc-macro-meta-span branch from 74e6edf to dbf4910 Compare May 12, 2021 05:01
@Aaron1011
Copy link
Member Author

@bors r=estebank

@bors
Copy link
Contributor

bors commented May 12, 2021

📌 Commit dbf4910 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2021
@bors
Copy link
Contributor

bors commented May 12, 2021

⌛ Testing commit dbf4910 with merge c1e7e36...

@petrochenkov
Copy link
Contributor

I'm still interested in reviewing this PR before it's merged, and understanding why it requires such invasive changes and keeping new data for proc macros.

@petrochenkov
Copy link
Contributor

r? @petrochenkov

@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 12, 2021
@bors
Copy link
Contributor

bors commented May 12, 2021

☀️ Test successful - checks-actions
Approved by: estebank
Pushing c1e7e36 to master...

@Aaron1011
Copy link
Member Author

@petrochenkov It looks like you commented just after I re-approved it, so it got merged anyway. Did you want to revert this, or are there some specific cleanups you'd like me to make in a follow-up PR?

@petrochenkov
Copy link
Contributor

I didn't look at this in detail yet, will look next weekend (since this is already merged and it's too late to hurry).

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2021
Cleanup span quoting

I finally got to reviewing rust-lang#84278.
See the individual commit messages.
r? `@Aaron1011`
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 merged-by-bors This PR was explicitly merged by bors. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants