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

Support trailing commas in lint_array #47428

Closed
theotherphil opened this issue Jan 14, 2018 · 6 comments · Fixed by #47458
Closed

Support trailing commas in lint_array #47428

theotherphil opened this issue Jan 14, 2018 · 6 comments · Fixed by #47458
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@theotherphil
Copy link
Contributor

rust-lang/rust-clippy#2350 (comment)

@kennytm kennytm added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Jan 14, 2018
@kennytm
Copy link
Member

kennytm commented Jan 14, 2018

Basically just change this macro to recognize and discard the trailing comma.

/// Declare a static `LintArray` and return it as an expression.
#[macro_export]
macro_rules! lint_array { ($( $lint:expr ),*) => (
{
static ARRAY: LintArray = &[ $( &$lint ),* ];
ARRAY
}
) }

@kennytm kennytm added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Jan 14, 2018
@durka
Copy link
Contributor

durka commented Jan 15, 2018

That macro declaration is formatted in a weird way. But anyway it should be enough to add $(,)* to the rule (unless accepting multiple trailing commas is undesired). I could mentor.

@mark-i-m
Copy link
Member

mark-i-m commented Jan 15, 2018

How about this:

macro_rules! lint_array { 
    ($( $lint:expr ),*,) => { lint_array!( $( &$lint ),* ) };
    ($( $lint:expr ),*) => {{ 
         static ARRAY: LintArray = &[ $( &$lint ),* ]; 
         ARRAY 
    }} 
} 

@mark-i-m
Copy link
Member

Out of curiosity, why doesn't rust allow trailing commas after $(pat),* automatically? Does it make it hard to write some macros? Alternately, why isn't there a ? (optional) repetition specification (e.g. $(pat)? would be 0 or 1 times)?

@durka
Copy link
Contributor

durka commented Jan 15, 2018 via email

@mark-i-m
Copy link
Member

Made a PR :)

mark-i-m added a commit to mark-i-m/rust that referenced this issue Jan 16, 2018
kennytm added a commit to kennytm/rust that referenced this issue Jan 17, 2018
kennytm added a commit to kennytm/rust that referenced this issue Jan 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants