aaron.ballman added inline comments.

================
Comment at: 
docs/clang-tidy/checks/readability-implicit-bool-conversion.rst:77-78
 
-- boolean expression/literal to integer,
+- boolean expression/literal to integer (conversion from boolean to a single
+  bit bitfield is explicitly allowed),
 
----------------
alexfh wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > I think it should only be allowed if the bit-field type is unsigned; 
> > > signed bit-fields with a single bit are inherently not portable because 
> > > you don't know if that bit represents a sign bit or a value bit (imagine 
> > > a sign and magnitude integer representation).
> > > 
> > > C++20 is changing this by standardizing on two's complement, but earlier 
> > > versions of C++ (and currently, all versions of C) are still impacted, so 
> > > another approach is to gate this on the language standard mode that's in 
> > > effect.
> > I think it's the responsibility of a compiler using sign and magnitude 
> > representation to warn about signed single bit bitfields.
> I agree with Malcolm's argument. But if the concern is practical (i.e. if 
> there's a user of this check, who's working with such compiler), we can add 
> an option to enable the warning in this case. Any objections?
I think LLVM only supports two's complement backends currently anyway, so this 
is probably fine as-is. It was more a reaction to "there is no information 
loss" because there is information loss with the integer value, even in two's 
complement. It seems like clang is missing a warning that could handle this, 
however. https://godbolt.org/z/34xXIQ

Boolean assignments are a bit of a different beast in that the `bool` is 
converted to integer as either `0` or `1`, explicitly. It's a bit strange that 
you put in `true` (which would be converted to `1`) and get back out `-1` as 
the integer value, but it's not awful because of the usual "0 is false, nonzero 
is true" conversion behavior back to `bool`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54941



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

Reply via email to