Prazek added a comment.

In http://reviews.llvm.org/D19105#422702, @Quuxplusone wrote:

> It seems like this proposed diagnostic and fixit, statistically speaking, is 
> *never* correct.
>  In the cases where there is a code issue to be corrected, the diagnosable 
> issue really seems to involve dataflow analysis:
>
> "Here is a variable of integral type (not bool), which over its lifetime 
> assumes only the values `0` and `1`. This variable should be declared as type 
> `bool`."
>  "Here is a function returning integral type (not bool), which returns only 
> the values `0` and `1`. This function should be declared as returning type 
> `bool`."
>  "Here is a parameter of integral type (not bool), which (via whole-program 
> analysis) assumes only the values `0` and `1`. This parameter should be 
> declared as type `bool`."


You are right, if someone is using true/false, then he probably mean it, and he 
is just assigning it to wrong type. But I don't see a point spending too much 
time to write something that would be smart enough to fix most of the issues as 
we would like. I think having this fixit is good for now and I would leave it 
like this. 
From my usage, I like when clang-tidy touch some files and changes it somewhow, 
so it is much simpler to check it in git than to look at the warnings.


================
Comment at: lib/Driver/Tools.cpp:2022
@@ -2021,3 +2021,3 @@
 
-    if (OptionIter->second == true) {
+    if (OptionIter->second == 1) {
       // Duplicate option specified.
----------------
Quuxplusone wrote:
> Shouldn't the original code already have triggered some warning about 
> explicit comparison with a boolean value?
> 
>     if (OptionIter->second) {
> 
> doesn't have literally the same semantics, but it's clearly IMHO what was 
> intended.
> Also, if `decltype(llvm::StringMap<bool>::iterator{}->second)` is not `bool`, 
> something weird is going on.
te misc-simplify-boolean-expression is to solve this problem. I will check if 
this code make sense.


http://reviews.llvm.org/D19105



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

Reply via email to