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

Allow proc macro to initialize a private field with a def_site value #47311

Closed
dtolnay opened this issue Jan 10, 2018 · 2 comments
Closed

Allow proc macro to initialize a private field with a def_site value #47311

dtolnay opened this issue Jan 10, 2018 · 2 comments
Assignees
Labels
A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Jan 10, 2018

In a struct initializer expression S { k: v } generated by a procedural macro where the field k is private, it seems both k and v need to be spanned with call_site in order for the generated code to compile. I believe only k should be required to be call_site, and v should be allowed to be def_site or anything else.

This currently blocks a correct implementation of derive(Deserialize).

#![feature(proc_macro)]

extern crate mac;

struct S {
    unit: (),
}

fn main() {
    // Expands to `S { unit: () }`.
    mac::init_field_unit!(S unit);
}
#![feature(proc_macro)]

extern crate proc_macro;
use proc_macro::{TokenStream, TokenTree, TokenNode, Delimiter, Spacing, Span};

#[proc_macro]
pub fn init_field_unit(input: TokenStream) -> TokenStream {
    let mut iter = input.into_iter();
    let struct_name = iter.next().unwrap();
    let field_name = iter.next().unwrap();
    assert!(iter.next().is_none());

    // Expands to `$struct_name { $field_name: () }`.
    vec![
        struct_name,
        TokenTree {
            span: Span::call_site(),
            kind: TokenNode::Group(Delimiter::Brace, vec![
                field_name,
                TokenTree {
                    span: Span::call_site(),
                    kind: TokenNode::Op(':', Spacing::Alone),
                },
                TokenTree {
                    // Works if this is call_site, but in derive(Deserialize) I
                    // need to be able to use def_site variables here.
                    span: Span::def_site(),
                    kind: TokenNode::Group(Delimiter::Parenthesis, TokenStream::empty()),
                },
            ].into_iter().collect()),
        },
    ].into_iter().collect()
}
error[E0451]: field `unit` of struct `S` is private
  --> src/main.rs:49:5
   |
49 |     mac::init_field_unit!(S unit);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ field `unit` is private

@jseyfried

@dtolnay
Copy link
Member Author

dtolnay commented Jan 10, 2018

Found a workaround: wrap the def_site value in a set of call_site parentheses. It should work without this workaround.

TokenTree {
    span: Span::call_site(),
    kind: TokenNode::Group(Delimiter::Parenthesis, vec![
        TokenTree {
            span: Span::def_site(),
            kind: TokenNode::Group(Delimiter::Parenthesis, TokenStream::empty()),
        },
    ].into_iter().collect()),
},

@jseyfried jseyfried self-assigned this Jan 10, 2018
sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 2, 2018
This was fairly recently rewritten, so it should in theory be the most
straightforward derive to port. Unfortunately, due to
rust-lang/rust#47311, it's obnoxiously hard to
actually construct a struct in a derive right now. We have to do hacky
workarounds until that is fixed.
sgrif added a commit to diesel-rs/diesel that referenced this issue Feb 3, 2018
This was fairly recently rewritten, so it should in theory be the most
straightforward derive to port. Unfortunately, due to
rust-lang/rust#47311, it's obnoxiously hard to
actually construct a struct in a derive right now. We have to do hacky
workarounds until that is fixed.
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.
@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-macros-2.0 Area: Declarative macros 2.0 (#39412) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 6, 2018
@jseyfried
Copy link
Contributor

@dtolnay Sorry for the delay!
Fixed in #48082.

Manishearth added a commit to Manishearth/rust that referenced this issue Feb 19, 2018
…ene, r=jseyfried

macros: improve struct constructor field hygiene, fix span bug

Fixes rust-lang#47311.
r? @nrc
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Mar 3, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 10, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 10, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 10, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 10, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 10, 2018
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-enhancement Category: An issue proposing an enhancement or a PR with one. 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

3 participants