-
-
Notifications
You must be signed in to change notification settings - Fork 717
COMP: suppress Clang -Wduplicate-enum #5422
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
COMP: suppress Clang -Wduplicate-enum #5422
Conversation
|
Interesting style change by clang-format. |
Mmmm, which style change? I added more #if branches, so things got reindented... |
|
I meant what clang-format wants by its complaint in pre-commit check. |
Oh! fugly. :) Odd, because I ran |
|
@seanm Thank you very much for addressing these warnings! I wasn't aware of them, when I proposed those enum's (PR #5410). By the way, it appears that an enumerator like
However, something like
As I observed at https://godbolt.org/z/bPTzc68YK using clang 20.1.0 But unfortunately the |
| # if __has_warning("-Wduplicate-enum") | ||
| # define ITK_CLANG_SUPPRESS_Wduplicate_enum ITK_PRAGMA(clang diagnostic ignored "-Wduplicate-enum") | ||
| # else | ||
| # define ITK_CLANG_SUPPRESS_Wduplicate_enum | ||
| # endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the if __has_warning is necessary to avoid another warning (warning: unknown warning group '-W...', ignored [-Wunknown-warning-option]). Unfortunately it's a bit cumbersome to have 5 lines of boilerplate code to define such a macro (ITK_CLANG_SUPPRESS_Wduplicate_enum), for just one warning. Would it be possible to make macro definitions like this more concise? And would it be possible to just specify the string "-Wduplicate-enum" once, instead of twice? I'm just asking. 🤷 No show stopper.
Also use __has_warning to guard againist trying to disable warnings that are unknown (ex: in older Clangs).
maybe I've fixed it with my change to the clang-format config file... |
N-Dekker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @seanm
If you ever find a way to define macro's like ITK_CLANG_SUPPRESS_Wduplicate_enum in a more concise manner, please let us know. Approved anyway 👍
thewtex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @seanm 🙏
To further elaborate: enum class MyEnum
{
UNKNOWN,
CHAR,
INT8 = CHAR, // Intended duplicate value, no problem, no warning!
WONDERFUL = 42, // OK, new enum value
DUPLICATE_ZERO = 0 // <== Warning! Duplicate value!
}; |
No description provided.