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

macros can't interpolate impl items (or really parse them at all) #48137

Closed
durka opened this issue Feb 11, 2018 · 3 comments · Fixed by #69366
Closed

macros can't interpolate impl items (or really parse them at all) #48137

durka opened this issue Feb 11, 2018 · 3 comments · Fixed by #69366
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-parser Area: The parsing of Rust source code to an AST 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

@durka
Copy link
Contributor

durka commented Feb 11, 2018

This macro will never work:

macro_rules! foo {
    ($name:ident; $method:item) => (
        struct $name();
        impl $name {
            $method
        }
    )
}

Instead of reinterpreting $method as an impl item, the macro expander says it was expecting const, crate, default, extern, fn, pub, type, unsafe, or }.

Moreover :item isn't good enough for parsing impl items because it doesn't have a special case for self arguments.

Is the solution to create a new :implitem or to make :item more flexible in parsing and expansion?

@petrochenkov petrochenkov added A-parser Area: The parsing of Rust source code to an AST A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Feb 11, 2018
@kennytm kennytm added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Feb 11, 2018
@petrochenkov
Copy link
Contributor

How we can solve this (I'm using functions as an example, but constants/types can be treated similarly):

  • Introduce cover grammar for free functions and functions in traits/impls/extern-blocks. This should require little changes, these grammars are very similar.
  • Use same structures in AST for functions and functions in traits/impls/extern-blocks.
  • Report a semantic error in AST validation if something inappropriate was parsed by the cover grammar, e.g. self in a free function, or body in a foreign function. This should also greatly improve error messages.
  • If we encounter an interpolated item (Nonterminal::NtItem) when trying to parse a traits/impls/extern-blocks item, accept it if it's a fn item and let AST validation care about everything else.

@petrochenkov petrochenkov removed the C-bug Category: This is a bug. label Feb 14, 2018
@petrochenkov petrochenkov self-assigned this Feb 14, 2018
@petrochenkov petrochenkov added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Feb 14, 2018
@steveklabnik
Copy link
Member

Triage:

macro_rules! foo {
    ($name:ident; $method:item) => (
        struct $name();
        impl $name {
            $method
        }
    )
}

foo!(Name, fn foo() { println!("Hello world"); });

gives

error: no rules expected the token `,`
  --> src/lib.rs:10:10
   |
1  | macro_rules! foo {
   | ---------------- when calling this macro
...
10 | foo!(Name, fn foo() { println!("Hello world"); });
   |          ^ no rules expected this token in macro call

@petrochenkov
Copy link
Contributor

The work on implementing #48137 (comment) has been steadily happening and #69366 almost completes it.

@bors bors closed this as completed in 79cd224 Feb 24, 2020
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, ..) A-parser Area: The parsing of Rust source code to an AST 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.

4 participants