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

$crate incorrectly substituted in attribute macro invocation containing bang macro invocation #62325

Closed
dtolnay opened this issue Jul 3, 2019 · 4 comments · Fixed by #62393
Closed
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Jul 3, 2019

When an attribute macro input contains $ident, it is correctly substituted both inside and outside of a function-like macro invocation i.e. both of the $ident's in [u8; $ident + m!($ident)] would be substituted as expected.

But when an attribute macro input contains $crate, it is only substituted outside of a function-like macro. Inside, it incorrectly disintegrates into Punct('$'), Ident(crate) when the whole bang macro invocation is passed to an attribute macro (or derive macro). In the case of [u8; $crate::N + m!($crate::N)] the first $crate would be handled correctly but the second one would be passed incorrectly as two tokens. See where I put // WRONG in the output below.


Cargo.toml

[package]
name = "repro"
version = "0.0.0"
edition = "2018"
publish = false

[lib]
proc-macro = true

src/main.rs

#[macro_export]
macro_rules! repro {
    ($ident:tt) => {
        #[repro::repro]
        type T = [$crate::$ident; x!($crate::$ident)];
    };
}

repro!(N);

fn main() {}

src/lib.rs

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_attribute]
pub fn repro(_args: TokenStream, input: TokenStream) -> TokenStream {
    println!("{:#?}", input);
    TokenStream::new()
}

Output of cargo check:

TokenStream [
    Ident {
        ident: "type",
        span: #0 bytes(0..0),
    },
    Ident {
        ident: "T",
        span: #0 bytes(0..0),
    },
    Punct {
        ch: '=',
        spacing: Alone,
        span: #0 bytes(0..0),
    },
    Group {
        delimiter: Bracket,
        stream: TokenStream [
            Ident {
                ident: "crate",
                span: #0 bytes(0..0),
            },
            Punct {
                ch: ':',
                spacing: Joint,
                span: #0 bytes(0..0),
            },
            Punct {
                ch: ':',
                spacing: Alone,
                span: #0 bytes(0..0),
            },
            Ident {
                ident: "N",
                span: #0 bytes(0..0),
            },
            Punct {
                ch: ';',
                spacing: Alone,
                span: #0 bytes(0..0),
            },
            Ident {
                ident: "x",
                span: #0 bytes(0..0),
            },
            Punct {
                ch: '!',
                spacing: Alone,
                span: #0 bytes(0..0),
            },
            Group {
                delimiter: Parenthesis,
                stream: TokenStream [
                    Punct {                              // WRONG
                        ch: '$',
                        spacing: Alone,
                        span: #0 bytes(0..0),
                    },
                    Ident {
                        ident: "crate",
                        span: #0 bytes(0..0),
                    },
                    Punct {
                        ch: ':',
                        spacing: Joint,
                        span: #0 bytes(0..0),
                    },
                    Punct {
                        ch: ':',
                        spacing: Alone,
                        span: #0 bytes(0..0),
                    },
                    Ident {
                        ident: "N",
                        span: #0 bytes(0..0),
                    },
                ],
                span: #0 bytes(0..0),
            },
        ],
        span: #0 bytes(0..0),
    },
    Punct {
        ch: ';',
        spacing: Alone,
        span: #0 bytes(0..0),
    },
]

Mentioning @petrochenkov who fixed my previous three $crate woes (#57089, #56622, #38016).

@dtolnay dtolnay added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jul 3, 2019
@petrochenkov petrochenkov self-assigned this Jul 3, 2019
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 3, 2019

This is #43081, ultimately.

The spans are dummy, as you can see, this means type T = ... goes through pretty-printing and back.

During pretty-printing we print $crate as crate/other_crate in path segments, but not in unstructured tokens (tokens inside x!(...) are unstructured tokens).

Now thinking about this again, I'm not sure why we cannot print it as crate/other_crate in unstructured tokens.
When implementing this pretty-printing, I assumed that would be incorrect for some reason.

@petrochenkov
Copy link
Contributor

I assumed that would be incorrect for some reason.

I recall it now.
Pretty-printing is the only way to determine the content of proc_macro::Ident.
If it converts $crate, then it becomes lossy and we no longer have way to figure out what the original identifier was.

Hmm, perhaps the conversion should be enabled only if the token printing routine is called from inside of AST printing, but not otherwise.

@petrochenkov
Copy link
Contributor

(I'll try to fix this tomorrow, shouldn't be too hard.)

@petrochenkov
Copy link
Contributor

Fixed in #62393

Centril added a commit to Centril/rust that referenced this issue Jul 10, 2019
…ulacrum

Fix pretty-printing of `$crate` (take 4)

Pretty-print `$crate` as `crate` or `crate_name` in unstructured tokens like `a $crate c` in `foo!(a $crate c)`, but only if those tokens are printed as a part of AST pretty-printing, rather than as a standalone token stream.

Fixes rust-lang#62325
Previous iterations - rust-lang#56647, rust-lang#57155, rust-lang#57915.
hawkw added a commit to tokio-rs/tracing that referenced this issue Aug 19, 2019
## Motivation


Currently, the `tracing` macros require curly braces as delimiters when
a `format_args` macro is used in addition to structured fields, like:

```rust
info!({ field1 = value1, field2 = value2 }, "unstructured message");
```

This is confusing, since the delimiters are not used in other cases; it
makes the syntax more complex; and, most importantly, I think it looks
kind of ugly. 

I've been planning to get rid of this when we transition to procedural
macros, but the transition is currently blocked on a compiler bug,
rust-lang/rust#62325.
(see #133 (comment))

I've been getting tired of waiting for this.

## Solution:

This branch updates the `tracing` crate's macros to support a
format_args message after the structured key-value fields without curly
braces. For example,

```rust
let yay = "WITHOUT DELIMITERS!!!";
info!(field1 = value1, field2 = value2, "message: {}", yay);
```

I've updated the tests & examples in the `tracing` crate so that they
show this usage rather than the old usage.

The old form with curly braces is still supported, since removing it
would be a breaking change, but we'll transition it out in examples &
tutorials. (can you put a `deprecated` attribute on a specific macro
arm???).

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) 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

Successfully merging a pull request may close this issue.

2 participants