[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added a comment. In D131255#3729304 , @aaron.ballman wrote: > After some more thought and some offline discussions, I think I have a > reasonable way forward: let's add `-Wsingle-bit-bitfield-constant-conversion` > as a new warning group

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added a comment. After some more thought and some offline discussions, I think I have a reasonable way forward: let's add `-Wsingle-bit-bitfield-constant-conversion` as a new warning group under `-Wbitfield-constant-conversion` that controls the diagnostic for one-bit bitfields.

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

bjope added a comment. FWIW, just some things noticed when I examined some of the new warning that popped up after this patch: https://github.com/llvm/llvm-project/issues/53253 mentioned that for example gcc complained about this. Although, as shown here https://godbolt.org/z/bq34Kexac there

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; ShawnZhong wrote: > aaron.ballman wrote: > >

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; aaron.ballman wrote: > aaron.ballman wrote: > >

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; aaron.ballman wrote: > ShawnZhong wrote: > >

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; ShawnZhong wrote: > ShawnZhong wrote: > >

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; ShawnZhong wrote: > ShawnZhong wrote: > >

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; aaron.ballman wrote: > thakis wrote: > > This was

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; thakis wrote: > This was to suppress false

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

thakis added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; This was to suppress false positives. All instances

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

This revision was automatically updated to reflect the committed changes. Closed by commit rG82afc9b169a6: Fix -Wbitfield-constant-conversion on 1-bit signed bitfield (authored by ShawnZhong, committed by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong marked an inline comment as done. ShawnZhong added a comment. In D131255#3706784 , @aaron.ballman wrote: > LGTM! Do you need someone to commit on your behalf? If so, what name and > email address would you like used for patch attribution?

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

erichkeane added a comment. I did a pass through, this seems like it is the right thing. I looked over the codegen tests, and I'm convinced it does what we want from that perspective. I'll leave it to @aaron.ballman to take another look for +1'ing this though. Repository: rG LLVM Github

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong updated this revision to Diff 450468. ShawnZhong added a comment. - Added CodeGen tests. - Fixed failed tests that assigned 1 to `int:1`. - Modified the Sema test to use `int:` instead of `signed char:1`. - Added release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added a comment. In D131255#3702286 , @ShawnZhong wrote: > Thanks for the quick reply and the reference on the C standard! > > On the C++ side, Section C.1.8 specified that `int` bit-fields are signed: > >> Change: Bit-fields of type plain

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong added a comment. Thanks for the quick reply and the reference on the C standard! On the C++ side, Section C.1.8 specified that `int` bit-fields are signed: > Change: Bit-fields of type plain int are signed. > Rationale: Leaving the choice of signedness to implementations could lead

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added reviewers: aaron.ballman, clang-language-wg. aaron.ballman added a comment. Thank you for working on this! I think this is going in the right direction, but I think there's some more work needed here because the rules depend on the type of the bit-field (at least in C, I've

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong updated this revision to Diff 450274. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131255/new/ https://reviews.llvm.org/D131255 Files: clang/lib/Sema/SemaChecking.cpp clang/test/Sema/constant-conversion.c Index:

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

ShawnZhong created this revision. ShawnZhong added a reviewer: rsmith. ShawnZhong added a project: clang. Herald added a project: All. ShawnZhong requested review of this revision. Herald added a subscriber: cfe-commits. Fix https://github.com/llvm/llvm-project/issues/53253 Repository: rG