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

[Shawn Zhong via Phabricator via cfe-commits]

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 LLVM Github Monorepo

https://reviews.llvm.org/D131255

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/constant-conversion.c


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -131,3 +131,15 @@
   s.a = -9;  // expected-warning{{implicit truncation from 'int' to bit-field 
changes value from -9 to 7}}
   s.a = 16;  // expected-warning{{implicit truncation from 'int' to bit-field 
changes value from 16 to 0}}
 }
+
+void test11(void) {
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -2; // expected-warning {{implicit conversion from 'int' to 'char' 
changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit conversion from 'int' to 'char' 
changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit conversion from 'int' to 'char' 
changes value from 2 to 0}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -131,3 +131,15 @@
   s.a = -9;  // expected-warning{{implicit truncation from 'int' to bit-field changes value from -9 to 7}}
   s.a = 16;  // expected-warning{{implicit truncation from 'int' to bit-field changes value from 16 to 0}}
 }
+
+void test11(void) {
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -2; // expected-warning {{implicit conversion from 'int' to 'char' changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit conversion from 'int' to 'char' changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit conversion from 'int' to 'char' changes value from 2 to 0}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Shawn Zhong via Phabricator via cfe-commits]

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: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -15,8 +15,16 @@
 }
 
 void test(void) {
-  struct { int bit : 1; } a;
-  a.bit = 1; // shouldn't warn
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -3; // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from -3 to -1}}
+  s.c = -2; // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit truncation from 'int' to bit-field 
changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 


Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -15,8 +15,16 @@
 }
 
 void test(void) {
-  struct { int bit : 1; } a;
-  a.bit = 1; // shouldn't warn
+  struct S {
+signed char c : 1;  // the only valid values are 0 and -1
+  } s;
+
+  s.c = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+  s.c = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+  s.c = -1; // no-warning
+  s.c = 0;  // no-warning
+  s.c = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  s.c = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13067,11 +13067,6 @@
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Shawn Zhong via Phabricator via cfe-commits]

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 to 
> inconsistent definitions of template specializations. For consistency, the 
> implementation freedom was eliminated for non-dependent types, too.
> Effect on original feature: The choise is implementation-defined in C, but 
> not so in C++.

Implementation-wise, I'll see what I can find in CodeGen on whether `int` 
bit-fields are signed for C


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

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

[Shawn Zhong via Phabricator via cfe-commits]

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 ACTION
  https://reviews.llvm.org/D131255/new/

https://reviews.llvm.org/D131255

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/class/class.bit/p1.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/CodeGen/bitfield.c
  clang/test/CodeGenCXX/bitfield-layout.cpp
  clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
  clang/test/Rewriter/rewrite-modern-struct-ivar.mm
  clang/test/Sema/constant-conversion.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/cxx1z-decomposition.cpp
  clang/test/SemaCXX/decomposition-blocks.cpp

Index: clang/test/SemaCXX/decomposition-blocks.cpp
===
--- clang/test/SemaCXX/decomposition-blocks.cpp
+++ clang/test/SemaCXX/decomposition-blocks.cpp
@@ -7,7 +7,7 @@
 
 void run(void (^)());
 void test() {
-  auto [i, j] = S{1, 42}; // expected-note {{'i' declared here}}
+  auto [i, j] = S{-1, 42}; // expected-note {{'i' declared here}}
   run(^{
 (void)i; // expected-error {{reference to local binding 'i' declared in enclosing function 'test'}}
   });
Index: clang/test/SemaCXX/cxx1z-decomposition.cpp
===
--- clang/test/SemaCXX/cxx1z-decomposition.cpp
+++ clang/test/SemaCXX/cxx1z-decomposition.cpp
@@ -84,7 +84,7 @@
 
   (void)[outerbit1]{}; // expected-error {{'outerbit1' cannot be captured because it does not have automatic storage duration}}
 
-  auto [bit, var] = S2{1, 1}; // expected-note 2{{'bit' declared here}}
+  auto [bit, var] = S2{-1, 1}; // expected-note 2{{'bit' declared here}}
 
   (void)[&bit] { // expected-error {{non-const reference cannot bind to bit-field 'a'}} \
 // expected-warning {{C++20}}
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2035,6 +2035,7 @@
   }
 };
 static_assert(X::f(3) == -1, "3 should truncate to -1");
+static_assert(X::f(1) == -1, "1 should truncate to -1");
   }
 
   struct HasUnnamedBitfield {
Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -15,8 +15,16 @@
 }
 
 void test(void) {
-  struct { int bit : 1; } a;
-  a.bit = 1; // shouldn't warn
+  struct S {
+int b : 1;  // The only valid values are 0 and -1.
+  } s;
+
+  s.b = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+  s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+  s.b = -1; // no-warning
+  s.b = 0;  // no-warning
+  s.b = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  s.b = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/test/Rewriter/rewrite-modern-struct-ivar.mm
===
--- clang/test/Rewriter/rewrite-modern-struct-ivar.mm
+++ clang/test/Rewriter/rewrite-modern-struct-ivar.mm
@@ -41,10 +41,10 @@
 @implementation Foo
 - (void)x {
   bar.x = 0;
-  bar.y = 1;
+  bar.y = -1;
 
   s.x = 0;
-  s.y = 1;
+  s.y = -1;
 }
 @end
 
Index: clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
===
--- clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
+++ clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
@@ -34,7 +34,7 @@
 @implementation Foo
 - (void)x:(Foo *)other {
   bar.x = 0;
-  bar.y = 1;
+  bar.y = -1;
   self->_internal._singleRange._range = (( other ->bar.x) ? &( other ->_internal._singleRange._range) : ((NSRange *)(&(((_NSRangeInfo *)( other ->_internal._multipleRanges._data))->_ranges[0];
 }
 @end
Index: clang/test/CodeGenCXX/bitfield-layout.cpp
===
--- clang/test/CodeGenCXX/bitfield-layout.cpp
+++ clang/test/CodeGenCXX/bitfield-layout.cpp
@@ -105,6 +105,15 @@
 // CHECK: define{{.*}} i32 @test_trunc_three_bits()
 // CHECK: ret i32 -1
 
+int test_trunc_one_bit() {
+  union {
+int i : 1; // truncated to 0b1 == -1
+  } const U = {1};  // 0b0001
+  return U.i;
+}
+// CHECK: define{{.*}} i32 @test_trunc_one_bit()
+// CHECK: ret i32 -1
+
 int test_trunc_1() {
   union {
 int i : 1; // truncated to 0b1 == -1
Index: clang/test/CodeGen/bitfield.c
===

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

[Shawn Zhong via Phabricator via cfe-commits]

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?

I'm new to Phabricator. I don't need someone else to commit on my behalf, but I 
guess I don't have permission to merge a commit on my own.

If the patch submitted misses author information for some reason, feel free to 
use `Shawn Zhong `.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

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

[Shawn Zhong via Phabricator via cfe-commits]

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 to suppress false positives. All instances we've seen of this are 
> > in fact false positives.
> > 
> > Was there any analysis on true / false positives for this change?
> > 
> > At least for our code, this seems to make the warning strictly worse.
> I've not performed any such analysis, but it would be good to know. FWIW, 
> this is the kind of situation I was thinking this diagnostic would help make 
> far more clear: https://godbolt.org/z/336n9xex3 (not that I expect people to 
> write that static assert, but I very much expect people who assign the value 
> 1 into a bit-field and then check for the value 1 to come back out are going 
> to be surprised).
> 
> That said, another approach to that specific scenario, which might have a 
> better true positive rate would be:
> ```
> struct S {
>   int bit : 1;
> };
> 
> int main() {
>   constexpr S s{1}; // No warning
>   if (s.bit == 1) { // Warn about explicit comparison of a 1-bit bit-field
> ...
>   } else if (s.bit == 0) { // Warn about explicit comparison of a 1-bit 
> bit-field?
> ...
>   } else if (s.bit) { // No warning
> ...
>   } else if (!s.bit) { // No warning
> ...
>   }
> }
> ```
Do you have something in mind that counts as false positives? 



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:
> > thakis wrote:
> > > This was to suppress false positives. All instances we've seen of this 
> > > are in fact false positives.
> > > 
> > > Was there any analysis on true / false positives for this change?
> > > 
> > > At least for our code, this seems to make the warning strictly worse.
> > I've not performed any such analysis, but it would be good to know. FWIW, 
> > this is the kind of situation I was thinking this diagnostic would help 
> > make far more clear: https://godbolt.org/z/336n9xex3 (not that I expect 
> > people to write that static assert, but I very much expect people who 
> > assign the value 1 into a bit-field and then check for the value 1 to come 
> > back out are going to be surprised).
> > 
> > That said, another approach to that specific scenario, which might have a 
> > better true positive rate would be:
> > ```
> > struct S {
> >   int bit : 1;
> > };
> > 
> > int main() {
> >   constexpr S s{1}; // No warning
> >   if (s.bit == 1) { // Warn about explicit comparison of a 1-bit bit-field
> > ...
> >   } else if (s.bit == 0) { // Warn about explicit comparison of a 1-bit 
> > bit-field?
> > ...
> >   } else if (s.bit) { // No warning
> > ...
> >   } else if (!s.bit) { // No warning
> > ...
> >   }
> > }
> > ```
> Do you have something in mind that counts as false positives? 
BTW, I realized that no warning is reported when a bool is assigned to a 
single-bit signed bit-field:

https://godbolt.org/z/M5ex48T8s

```
int main() {
  struct S { int b : 1; } s;
  s.b = true;
  if (s.b == true) { puts("T"); } else { puts("F"); }
}
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

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

[Shawn Zhong via Phabricator via cfe-commits]

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:
> > aaron.ballman wrote:
> > > thakis wrote:
> > > > This was to suppress false positives. All instances we've seen of this 
> > > > are in fact false positives.
> > > > 
> > > > Was there any analysis on true / false positives for this change?
> > > > 
> > > > At least for our code, this seems to make the warning strictly worse.
> > > I've not performed any such analysis, but it would be good to know. FWIW, 
> > > this is the kind of situation I was thinking this diagnostic would help 
> > > make far more clear: https://godbolt.org/z/336n9xex3 (not that I expect 
> > > people to write that static assert, but I very much expect people who 
> > > assign the value 1 into a bit-field and then check for the value 1 to 
> > > come back out are going to be surprised).
> > > 
> > > That said, another approach to that specific scenario, which might have a 
> > > better true positive rate would be:
> > > ```
> > > struct S {
> > >   int bit : 1;
> > > };
> > > 
> > > int main() {
> > >   constexpr S s{1}; // No warning
> > >   if (s.bit == 1) { // Warn about explicit comparison of a 1-bit bit-field
> > > ...
> > >   } else if (s.bit == 0) { // Warn about explicit comparison of a 1-bit 
> > > bit-field?
> > > ...
> > >   } else if (s.bit) { // No warning
> > > ...
> > >   } else if (!s.bit) { // No warning
> > > ...
> > >   }
> > > }
> > > ```
> > Do you have something in mind that counts as false positives? 
> BTW, I realized that no warning is reported when a bool is assigned to a 
> single-bit signed bit-field:
> 
> https://godbolt.org/z/M5ex48T8s
> 
> ```
> int main() {
>   struct S { int b : 1; } s;
>   s.b = true;
>   if (s.b == true) { puts("T"); } else { puts("F"); }
> }
> ```
> 
> 
For reference, I found this issue on chromium: 

https://bugs.chromium.org/p/chromium/issues/detail?id=1352183


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

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

[Shawn Zhong via Phabricator via cfe-commits]

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:
> > ShawnZhong wrote:
> > > ShawnZhong wrote:
> > > > ShawnZhong wrote:
> > > > > aaron.ballman wrote:
> > > > > > thakis wrote:
> > > > > > > This was to suppress false positives. All instances we've seen of 
> > > > > > > this are in fact false positives.
> > > > > > > 
> > > > > > > Was there any analysis on true / false positives for this change?
> > > > > > > 
> > > > > > > At least for our code, this seems to make the warning strictly 
> > > > > > > worse.
> > > > > > I've not performed any such analysis, but it would be good to know. 
> > > > > > FWIW, this is the kind of situation I was thinking this diagnostic 
> > > > > > would help make far more clear: https://godbolt.org/z/336n9xex3 
> > > > > > (not that I expect people to write that static assert, but I very 
> > > > > > much expect people who assign the value 1 into a bit-field and then 
> > > > > > check for the value 1 to come back out are going to be surprised).
> > > > > > 
> > > > > > That said, another approach to that specific scenario, which might 
> > > > > > have a better true positive rate would be:
> > > > > > ```
> > > > > > struct S {
> > > > > >   int bit : 1;
> > > > > > };
> > > > > > 
> > > > > > int main() {
> > > > > >   constexpr S s{1}; // No warning
> > > > > >   if (s.bit == 1) { // Warn about explicit comparison of a 1-bit 
> > > > > > bit-field
> > > > > > ...
> > > > > >   } else if (s.bit == 0) { // Warn about explicit comparison of a 
> > > > > > 1-bit bit-field?
> > > > > > ...
> > > > > >   } else if (s.bit) { // No warning
> > > > > > ...
> > > > > >   } else if (!s.bit) { // No warning
> > > > > > ...
> > > > > >   }
> > > > > > }
> > > > > > ```
> > > > > Do you have something in mind that counts as false positives? 
> > > > BTW, I realized that no warning is reported when a bool is assigned to 
> > > > a single-bit signed bit-field:
> > > > 
> > > > https://godbolt.org/z/M5ex48T8s
> > > > 
> > > > ```
> > > > int main() {
> > > >   struct S { int b : 1; } s;
> > > >   s.b = true;
> > > >   if (s.b == true) { puts("T"); } else { puts("F"); }
> > > > }
> > > > ```
> > > > 
> > > > 
> > > For reference, I found this issue on chromium: 
> > > 
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=1352183
> > > Do you have something in mind that counts as false positives?
> > 
> > In my example above, I consider the use of `s.bit` and `!s.bit` to count as 
> > false positives. The value in the bit-field being 1 or -1 has no bearing on 
> > the semantics of the program, the only thing that matters is zero vs 
> > nonzero because of the boolean conversion.
> > 
> > > BTW, I realized that no warning is reported when a bool is assigned to a 
> > > single-bit signed bit-field:
> > 
> > Good catch, the conversion from bool to integer does give the value `1` 
> > explicitly. At the same time, I would consider the assignment to the 
> > bit-field from a boolean to be a non-issue, the problem only stems from 
> > attempting to use inspect the value.
> >>BTW, I realized that no warning is reported when a bool is assigned to a 
> >>single-bit signed bit-field:
> > Good catch, the conversion from bool to integer does give the value 1 
> > explicitly. At the same time, I would consider the assignment to the 
> > bit-field from a boolean to be a non-issue, the problem only stems from 
> > attempting to use inspect the value.
> 
> Actually, that's a more interesting case than I had originally realized. See 
> C2x 6.7.2.1p12. "If the value 0 or 1 is stored into a nonzero-width bit-field 
> of type bool, the value of the bit-field shall compare equal to the value 
> stored; a bool bit-field has the semantics of a bool."
> 
> So if you store a 1 into a bool bit-field, you have to get a 1 back out of 
> it. The bit-field is implicitly unsigned, effectively.
> In my example above, I consider the use of `s.bit` and `!s.bit` to count as 
> false positives. The value in the bit-field being 1 or -1 has no bearing on 
> the semantics of the program, the only thing that matters is zero vs nonzero 
> because of the boolean conversion.

I agree with you that if `s.bit` is only used to be converted to `bool`, then 
the value stored being 1 or -1 does not matter that much. But hypothetically, 
one could use `s.bit` for arithmetic (e.g., index into an array), assignment 
(say into another int or enum), or, in your example, comparison against another 
value.  If a warning is not generated during conversion, handling them in other 
places would be harder. 

> Actually, that's a more interesting case than I had originally realized. See 
> C2x 6.7.2.1p12. "If the value 0 or 1 is stored into a nonzero-width bit-field 
> of type bool, the va