[PATCH] D19105: Changes in clang after running http://reviews.llvm.org/D18821

[Piotr Padlewski via cfe-commits]

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