-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fixed all clang -Wsigned-enum-bitfield warnings #2882
Conversation
606b63b
to
c92da16
Compare
include/fmt/core.h
Outdated
@@ -2029,14 +2029,20 @@ template <typename Context> class basic_format_args { | |||
// between clang and gcc on ARM (#1919). | |||
using format_args = basic_format_args<format_context>; | |||
|
|||
// We cannot use enum classes as bit fields because of a gcc bug | |||
// We cannot use enum classes as bit fields because of a gcc bug before 9.3 |
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.
The issue with pre-9.3 gcc version seems to be about underlying types rather than enum classes because we don't use the former in both branches below. So the part about 9.3 should probably not be folded in the original comment which belongs in front of namespace align
to explain why we use namespace + enum instead of scoped enums.
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'm not so sure they are different bugs in fact. But in any case I've improved both the comments in the code and the commit message.
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!
Could you provide a repro (ideally a gobolt link) for future reference. |
There's one in the previous PR: #2801 (comment) here: https://godbolt.org/z/58aEv8zEq Or do you mean to add that link in the code comments? |
No, I meant in the description of this PR so that it contains all the relevant info. |
Made enums involved in bitfields unsigned by specifying their underlying type as unsigned char. Due to a bug, when specifying an underlying type, gcc < 9.3 warns about bitfields not being big enough to hold the enum, even though they are. So keep the plain enum for old gcc. An example of the bug is here: https://godbolt.org/z/58aEv8zEq
c92da16
to
6dfb3c3
Compare
Merged, thank you! |
@vitaut any chance there is a release / tag coming soon? These warnings I fixed I'd like to integrate into another project I work on, but they prefer to reference official tags / releases of their upstream projects (like fmtlib)... |
Yes by some definition of "soon". I don't have any specific dates though. |
Made enums involved in bitfields unsigned by specifying their underlying type as unsigned char.
Due to a bug, when specifying an underlying type, gcc < 9.3 warns about bitfields not being big enough to hold the enum, even though they are. So keep the plain enum for old gcc.
An example of the bug is here:
https://godbolt.org/z/58aEv8zEq