Add trailing comma support to matches macro

[chrisduerr]

No description provided.

[rust-highfive]

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

[mark-i-m]

LGTM

[matklad]

Why do we want to allow comma here? There’s no comma separated list in the macro.

[chrisduerr]

Why do we want to allow comma here? There’s no comma separated list in the macro.

I would like trailing comma support for formatting purposes. Similar to how it is possible in functions to add trailing commas even though there is a fixed number of parameters. Since the parameters of this macro can get quite long, I don't think it's unusual to have them formatted in multiple lines like this:

if matches!(
    mything,
    Some(thing) if thing < a_pretty_long_variable_name,
) {
    // ...
}

[matklad]

I don't have a strong opinion here, but I still think this doesn't really make sense. The value of trailing commas in lists is that you can add/remove and reorder entries without adjusting commas.

Int this case, there are no entries to reorder, the syntactic categories of two things are different.

[chrisduerr]

@matklad Especially when it comes to code formatting, it makes sense to keep things consistent. With the argument that it only makes sense when there is a need to reorder parameters, then there's likely no need to have trailing commas for functions of STD that have different types for all their parameters. But chances are many people will still add trailing commas for multiline functions just so they don't have to think about it.

If I want to format this macro, I run into problems with rustfmt, but what I do is just remove the ! and then format it as a function so I don't have to think about things. That will put a , at the end of the second parameter, for uniformity. I then change it back to a macro and run into an error. Then when my code is reviewed, someone notices this missing uniformity and mentions that there should be a trailing comma there, just to find out that it's not possible. This is how I got aware of this "problem".

Is it functionally necessary to have a trailing comma here? I don't want to say there's no purpose at all, but the main reasons why trailing commas are allowed don't really apply here. Will it reduce the friction of writing code when using this macro? I think it does and I think that makes it worth it to add.

[nikomatsakis]

I'm in favor, I think my brain would "pattern match" on the fact that this is a (...) separated list and wish for the second comma. Also, I don't really see the distinction between fn foo(a: u32, b: u32) and matches!, both are "things that take two arguments" in some sense, but we allow trailing commas for foo (even though it would never make sense to add a third argument, as @chrisduerr noted).

(Of course, you could edit the source of foo to make it make sense, but by the same token we might some day modify matches! to add an optional third parameter...)

[Dylan-DPC-zz]

merging this based on #74880 (comment)

@bors r=nikomatsakis rollup

[bors]

📌 Commit f3d7645 has been approved by nikomatsakis

[matklad]

FWIW, I would be more comfortable if someone from T-libs looked at this (as this formally changes the public API of std), but I also don’t think this really matters.

[matklad]

hm, no, maybe I am just paranoid, but let’s ping @rust-lang/libs to be sure :-)

[Dylan-DPC-zz]

fair

@bors r-

will r+ once libs is fine with it

[dtolnay]

I am on board with this. When used without an if guard, the invocation (often) looks syntactically like a pair of expressions, so it makes sense to support formatting it the conventional way that we format a line wrapped pair of expressions.

[dtolnay]

@bors r+

[bors]

📌 Commit f3d7645 has been approved by dtolnay