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

Derive for items defined inside a function receive incorrect span information #47983

Closed
sgrif opened this issue Feb 3, 2018 · 2 comments
Closed
Labels
A-attributes Area: #[attributes(..)] A-macros-1.2 Area: Declarative macros 1.2 A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sgrif
Copy link
Contributor

sgrif commented Feb 3, 2018

Steps to reproduce (rustc 1.25.0-nightly (56733bc9f 2018-02-01))

src/main.rs

#![feature(proc_macro)]

extern crate repro;
use repro::AsChangeset;

fn main() {
    #[derive(AsChangeset)]
    #[table_name = "users"]
    struct UserForm {
        id: i32,
        name: String,
    }
}

src/lib.rs

#![feature(proc_macro)]

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_derive(AsChangeset, attributes(table_name))]
pub fn derive(input: TokenStream) -> TokenStream {
    for tt in input {
        println!("{:#?}", tt);
    }
    TokenStream::empty()
}

The struct token will have an incorrect span which points at the correct source code, but shows <macro expansion>:1:1 instead of a file/line number when used. Every other token will have a useless span Span { lo: BytePos(0), hi: BytePos(0), ctxt: #0 }

@sgrif sgrif added A-attributes Area: #[attributes(..)] T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-bug Category: This is a bug. labels Feb 3, 2018
@sgrif
Copy link
Contributor Author

sgrif commented Feb 3, 2018

There also appears to be a bug when one of these dummy spans is received. Span::resolved_at appears to do absolutely nothing.

sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 3, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 4, 2018
Since `QueryableByName` is one of the more recently written derives, it
should have been a really straightforward port. Unfortunately, the
tests for this derive hit multiple rustc bugs

- rust-lang/rust#47983
- rust-lang/rust#47311

I love what we were able to do with the error message here. We could
even go so far as to have the `help` lines point at the struct itself
for the `table_name` annotation if we want to.

I also much prefer the workaround for
rust-lang/rust#47311 in this PR to the one I
did in #1529. I'll need to update that PR if this is merged first.
@alexcrichton alexcrichton added the A-macros-1.2 Area: Declarative macros 1.2 label May 22, 2018
@alexcrichton
Copy link
Member

This is caused by #43081, but I'm not precisely sure why yet

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jul 22, 2018
Ever plagued by rust-lang#43081 the compiler can return surprising spans in situations
related to procedural macros. This is exhibited by rust-lang#47983 where whenever a
procedural macro is invoked in a nested item context it would fail to have
correct span information.

While rust-lang#43230 provided a "hack" to cache the token stream used for each item in
the compiler it's not a full-blown solution. This commit continues to extend
this "hack" a bit more to work for nested items.

Previously in the parser the `parse_item` method would collect the tokens for an
item into a cache on the item itself. It turned out, however, that nested items
were parsed through the `parse_item_` method, so they didn't receive similar
treatment. To remedy this situation the hook for collecting tokens was moved
into `parse_item_` instead of `parse_item`.

Afterwards the token collection scheme was updated to support nested collection
of tokens. This is implemented by tracking `TokenStream` tokens instead of
`TokenTree` to allow for collecting items into streams at intermediate layers
and having them interleaved in the upper layers.

All in all, this...

Closes rust-lang#47983
bors added a commit that referenced this issue Jul 24, 2018
rustc: Implement tokenization of nested items

Ever plagued by #43081 the compiler can return surprising spans in situations
related to procedural macros. This is exhibited by #47983 where whenever a
procedural macro is invoked in a nested item context it would fail to have
correct span information.

While #43230 provided a "hack" to cache the token stream used for each item in
the compiler it's not a full-blown solution. This commit continues to extend
this "hack" a bit more to work for nested items.

Previously in the parser the `parse_item` method would collect the tokens for an
item into a cache on the item itself. It turned out, however, that nested items
were parsed through the `parse_item_` method, so they didn't receive similar
treatment. To remedy this situation the hook for collecting tokens was moved
into `parse_item_` instead of `parse_item`.

Afterwards the token collection scheme was updated to support nested collection
of tokens. This is implemented by tracking `TokenStream` tokens instead of
`TokenTree` to allow for collecting items into streams at intermediate layers
and having them interleaved in the upper layers.

All in all, this...

Closes #47983
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: #[attributes(..)] A-macros-1.2 Area: Declarative macros 1.2 A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-bug Category: This is a bug. 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

2 participants