bjope added inline comments.

================
Comment at: clang/test/Sema/constant-conversion.c:30
+  s.b = 1;     // one-bit-warning {{implicit truncation from 'int' to a 
one-bit wide bit-field changes value from 1 to -1}}
+  s.b = true;  // no-warning (we suppress it manually to reduce false 
positives)
+  s.b = false; // no-warning
----------------
(Sorry for being late to the party, with post commit comments. I'm just trying 
to understand the reasoning about always suppressing the warning based on 
"true".)

Isn't the code a bit suspicious when using true/false from stdbool.h, while 
using a signed bitfield?

When doing things like 
```
(s.b == true) ? 1 : 0
```
the result would always be zero if s.b is a signed 1-bit bitfield.

So wouldn't it make more sense to actually use an unsigned bitfield (such as 
the bool type from stdbool.h) when the intent is to store a boolean value and 
using defines from stdbool.h?

Is perhaps the idea that we will get warnings about `(s.b == true)` being 
redundant in situations like this, and then we do not need a warning on the 
bitfield assignment? Such a reasoning would actually make some sense, since 
`(s.b == true)` never would be true even when the bitfield is assigned a 
non-constant value, so we can't rely on catching the problem by only looking at 
bitfield assignments involving true/false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132851

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to