Skip to content

Support trailing commas in lint_array #47428

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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
# for free to join this conversation on GitHub. Already have an account? # 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