Extract a new warning `-Wc++98-compat-enumerator-comma` from `-Wc++98-compat-pedantic`

[SunBlack]

For stylistic reasons, we want it to be consistently in the code that enumerators do not have an extra comma at the end.

So he should warn about the extra , here, e.g.,:

enum MyEnum
{
	A,
	B,
};

-Wc++98-compat-pedantic contains a warning for this:

warning: commas at the end of enumerator lists are incompatible with C++98

But there exists no way to say I only want the warning for the unnecessary comma, but not the other warnings. So a warning -Wc++98-compat-enumerator-comma would be nice.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: SunBlack (SunBlack)

For stylistic reasons, we want it to be consistently in the code that enumerators do not have an extra comma at the end.

So he should warn about the extra , here, e.g.,:

enum MyEnum
{
	A,
	B,
};

-Wc++98-compat-pedantic contains a warning for this:

warning: commas at the end of enumerator lists are incompatible with C++98

But there exists no way to say I only want the warning for the unnecessary comma, but not the other warnings. So a warning -Wc++98-compat-enumerator-semi would be nice, which is then also included in -Wextra-semi.

[EugeneZelenko]

Should be different option, because semicolon (semi) is not same as comma.

[SunBlack]

Ah true, adjusted the issue

[shafik]

I believe both having he comma and leaving it out are pretty common. It is legal either way and so any diagnostic here would be quite noisy and not really beneficial.

[SunBlack]

As there exists already a warning, I think this doesn't change anything, when the warning got splitted from -Wc++98-compat-pedantic.

[AaronBallman]

I'm not convinced the new diagnostic group would be enabled often enough to be worth the extra maintenance costs for a stylistic warning. We usually put style-like warnings into clang-tidy, that seems like a better place for this kind of diagnostic.

[SunBlack]

Ah okay, I can't estimate the effort that splitting would require. But wouldn't Clang-Format be a better choice than Clang-Tidy? Then you could offer three options:

• No customization
• Always put a comma at the last option
• Never set a comma for the last option

It also has the advantage that it is applied directly by most developers, as Clang-Format is integrated in many IDEs, while Clang-Tidy is often only executed within a CI.

[AaronBallman]

Ah okay, I can't estimate the effort that splitting would require.

The effort to split the diagnostics is minimal. The real problem is that we don't typically add new off-by-default diagnostics because of how infrequently they're enabled by users in practice. It means we're creating additional diagnostic groups, have additional configurations to test, etc for a feature that's generally not used. So we typically ask that a diagnostic be something we can enable by default, or have a very compelling reason to exist.

But wouldn't Clang-Format be a better choice than Clang-Tidy?

clang-format is a bit of a question mark (CC @mydeveloperday @owenca); usually that's about formatting the existing code rather than inserting/removing code aside from whitespace.

[owenca]

But wouldn't Clang-Format be a better choice than Clang-Tidy?

clang-format is a bit of a question mark (CC @mydeveloperday @owenca); usually that's about formatting the existing code rather than inserting/removing code aside from whitespace.

I'm okay with adding such an option as we already have options for inserting braces and removing braces/semicolon/parentheses.

Ah okay, I can't estimate the effort that splitting would require. But wouldn't Clang-Format be a better choice than Clang-Tidy? Then you could offer three options:

• No customization
• Always put a comma at the last option
• Never set a comma for the last option

It also has the advantage that it is applied directly by most developers, as Clang-Format is integrated in many IDEs, while Clang-Tidy is often only executed within a CI.

@SunBlack can you open a new issue for the above feature and assign it to me?