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 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
    ...
  }
}
```


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

Reply via email to