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

Remove $:meta matcher in decl macros #49629

Closed
dtolnay opened this issue Apr 3, 2018 · 7 comments · Fixed by #63674
Closed

Remove $:meta matcher in decl macros #49629

dtolnay opened this issue Apr 3, 2018 · 7 comments · Fixed by #63674
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 Apr 3, 2018

$:meta is intended for matching the content of attributes, as in the following:

#![feature(decl_macro)]

macro m(
    $(#[$m:meta])*
    struct $n:ident;
) {
    $(#[$m])*
    struct $n(usize);
}

m! {
    #[derive(Debug)]
    struct S;
}

fn main() {
    println!("{:?}", S(1));
}

According to #34981 (comment) we intend to support arbitrary token streams inside of attributes rather than the limited grammar of $:meta. Let's have decl macros drop support for $:meta and encourage #[$($meta:tt)*] instead.

@dtolnay dtolnay added the A-macros-2.0 Area: Declarative macros 2.0 (#39412) label Apr 3, 2018
@eddyb
Copy link
Member

eddyb commented Apr 3, 2018

The less fragment specifiers the better! We might even want to change the macro_rules meta to ident followed by any number of tt's, just to keep things simple.

@dtolnay
Copy link
Member Author

dtolnay commented Apr 3, 2018

Changing macro_rules's meta is tricky because it is currently allowed to be followed by tokens.

macro_rules! eddyb {
    ($m:meta $n:tt) => {}
}

eddyb!(a ~);

@eddyb
Copy link
Member

eddyb commented Apr 3, 2018

So, the only generalization we can make is to have meta allow ident = tt and ident(tt*)?

@petrochenkov
Copy link
Contributor

we intend to support arbitrary token streams inside of attributes

I thought the idea was to support attribute invocations identical to bang macro invocations

a::b::c!(tt*)
a::b::c![tt*]
a::b::c!{tt*}

=>

#[a::b::c(tt*)]
#[a::b::c[tt*]]
#[a::b::c{tt*}]

+ two "legacy" forms

#[a::b::c] // Maybe not so "legacy" with https://internals.rust-lang.org/t/idea-elide-parens-brackets-on-unparametrized-macros/6527
#[a::b::c = not_sure_what_exactly_but_probably_tt]

, but not arbitrary "token streams" (aka tt* without delimiters)

#[a::b::c e f + c ,,, ;_:] // Nope

In this case, I think, we can fit it into existing meta with all its following token rules.

@pietroalbini pietroalbini added 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. labels Apr 5, 2018
@sgrif
Copy link
Contributor

sgrif commented Aug 2, 2018

This seems like something that should get done for Rust 2018 -- Should this be nominated for more discussion?

@petrochenkov
Copy link
Contributor

#50120 restricted the attribute grammar to delimited token trees, so it's at least technically possible to keep meta as a matcher now.

@petrochenkov
Copy link
Contributor

#63674 supports the modern attribute syntax in meta.

bors added a commit that referenced this issue Aug 17, 2019
syntax: Support modern attribute syntax in the `meta` matcher

Where "modern" means #57367:
```
PATH
PATH `(` TOKEN_STREAM `)`
PATH `[` TOKEN_STREAM `]`
PATH `{` TOKEN_STREAM `}`
```

Unfortunately, `meta` wasn't future-proofed using the `FOLLOW` token set like other matchers (#34011), so code like `$meta:meta {` or `$meta:meta [` may break, and we need a crater run to find out how often this happens in practice.

Closes #49629 (by fully supporting `meta` rather than removing it.)
Centril added a commit to Centril/rust that referenced this issue Oct 1, 2019
syntax: Support modern attribute syntax in the `meta` matcher

Where "modern" means rust-lang#57367:
```
PATH
PATH `(` TOKEN_STREAM `)`
PATH `[` TOKEN_STREAM `]`
PATH `{` TOKEN_STREAM `}`
```

Unfortunately, `meta` wasn't future-proofed using the `FOLLOW` token set like other matchers (rust-lang#34011), so code like `$meta:meta {` or `$meta:meta [` may break, and we need a crater run to find out how often this happens in practice.

Closes rust-lang#49629 (by fully supporting `meta` rather than removing it.)
@bors bors closed this as completed in 64130fd Oct 1, 2019
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

Successfully merging a pull request may close this issue.

5 participants