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

Fri Aug 5 06:00:23 PDT 2022

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 not checked C++ yet). See C2x 6.7.2.1p12 (and it's footnote): "A bit-field is interpreted as having a signed or unsigned integer type consisting of the specified number of bits. ..." (and the footnote goes on to say "As specified in 6.7.2 above, if the actual type specifier used is `int` or a typedef-name defined as `int`, then it is implementation-defined whether the bit-field is signed or unsigned. ...").

Assuming that we actually do treat plain `int` bit-fields as being signed (which I'm pretty sure we do), then I agree that these changes are fine. But you should add some CodeGen test coverage (or find existing coverage) that proves how we behave in that case and an `int` test in `Sema` showing we match the codegen behavior.

Also, can you add a release note for the fix?

================
Comment at: clang/test/Sema/constant-conversion.c:137
+  struct S {
+    signed char c : 1;  // the only valid values are 0 and -1
+  } s;
----------------

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131255/new/

https://reviews.llvm.org/D131255