Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. In http://reviews.llvm.org/D18821#403117, @Prazek wrote: > In http://reviews.llvm.org/D18821#402686, @Prazek wrote: > > > In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > > > > > BTW, why is the check in the 'modernize' module? It doesn't seem to make > > > anything more modern. I would guess, the pattern it detects is most > > > likely to result from a programming error. Also, the fix, though it > > > retains the behavior, has a high chance to be incorrect. Can you share > > > the results of running this check on LLVM? At least, how many problems it > > > found and how many times the suggested fix was correct. > > > > > > I'd suggest to move the check to `misc` or maybe it's time to create a > > > separate directory for checks targeting various bug-prone patterns. > > > > > > Do you have any thought about the name for such a module? I belive that > > misc is overloaded. > > > > So for this we are looking for something that is probably not a bug, but it > > makes code a little bit inaccurate > > Maybe something like: > > > > - accuracy, > > - correctness, > > - certainity, > > - safety, > > - maybebugmaybenothardtosay > > > after a long though I think that "accuracy" is the best name here - we want > to look for a code that is valid, but not accurate There are many possible reasons this pattern can appear in the code. Sometimes it's a bug, sometimes, it's an attempt to make the code look better than it is ;) It seems to me though, that having a type mismatch of this kind is a bug-prone pattern, so I would start a more generic "bugprone" category of checks (and move some misc- checks there later on). Also, please re-upload http://reviews.llvm.org/D19105 after disabling matches on 1-bit bitfields in the check. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. In http://reviews.llvm.org/D18821#402686, @Prazek wrote: > In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > > > BTW, why is the check in the 'modernize' module? It doesn't seem to make > > anything more modern. I would guess, the pattern it detects is most likely > > to result from a programming error. Also, the fix, though it retains the > > behavior, has a high chance to be incorrect. Can you share the results of > > running this check on LLVM? At least, how many problems it found and how > > many times the suggested fix was correct. > > > > I'd suggest to move the check to `misc` or maybe it's time to create a > > separate directory for checks targeting various bug-prone patterns. > > > Do you have any thought about the name for such a module? I belive that misc > is overloaded. > > So for this we are looking for something that is probably not a bug, but it > makes code a little bit inaccurate > Maybe something like: > > - accuracy, > - correctness, > - certainity, > - safety, > - maybebugmaybenothardtosay after a long though I think that "accuracy" is the best name here - we want to look for a code that is valid, but not accurate http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. In http://reviews.llvm.org/D18821#403103, @Quuxplusone wrote: > I would like to see a new version of http://reviews.llvm.org/D19105 with all > the "1-bit-bitfield" diffs removed. > Right now, it's hard to see that there's *anything* in > http://reviews.llvm.org/D19105 that's not a miscorrection of a 1-bit bitfield. > > Do you have an example of a large codebase where this check runs with few > false positives and a non-zero number of true positives? I will post fixex tomorrow. I don't think there will be any false or true positives in warnings, but the main problems with fixes are that many times developer wants to use bools, but it decalres field/return type as int. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Quuxplusone added a subscriber: Quuxplusone. Quuxplusone added a comment. I would like to see a new version of http://reviews.llvm.org/D19105 with all the "1-bit-bitfield" diffs removed. Right now, it's hard to see that there's *anything* in http://reviews.llvm.org/D19105 that's not a miscorrection of a 1-bit bitfield. Do you have an example of a large codebase where this check runs with few false positives and a non-zero number of true positives? http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > BTW, why is the check in the 'modernize' module? It doesn't seem to make > anything more modern. I would guess, the pattern it detects is most likely to > result from a programming error. Also, the fix, though it retains the > behavior, has a high chance to be incorrect. Can you share the results of > running this check on LLVM? At least, how many problems it found and how many > times the suggested fix was correct. > > I'd suggest to move the check to `misc` or maybe it's time to create a > separate directory for checks targeting various bug-prone patterns. Do you have any thought about the name for such a module? I belive that misc is overloaded. So for this we are looking for something that is probably not a bug, but it makes code a little bit inaccurate Maybe something like: - accuracy, - correctness, - certainity, - safety, - maybebugmaybenothardtosay http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 53909.
Prazek marked an inline comment as done.
Prazek added a comment.
I will think name for new module that would have all the checks like this.
I added ingnoring of bitfields of size 1
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
test/clang-tidy/modernize-bool-to-integer-conversion.cpp
Index: test/clang-tidy/modernize-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-bool-to-integer-conversion.cpp
@@ -0,0 +1,78 @@
+// RUN: %check_clang_tidy %s modernize-bool-to-integer-conversion %t
+
+const int is42Answer = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicitly converting bool literal to 'int'; use integer literal instead [modernize-bool-to-integer-conversion]
+// CHECK-FIXES: const int is42Answer = 1;{{$}}
+
+volatile int noItsNot = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicitly converting bool literal to 'int'; {{..}}
+// CHECK-FIXES: volatile int noItsNot = 0;{{$}}
+int a = 42;
+int az = a;
+
+long long ll = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting bool literal to 'long long';{{..}}
+// CHECK-FIXES: long long ll = 1;{{$}}
+
+void fun(int) {}
+#define ONE true
+
+// No fixup for macros.
+int one = ONE;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'int' inside a macro; use integer literal instead [modernize-bool-to-integer-conversion]
+
+void test() {
+ fun(ONE);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly converting bool{{..}}
+
+ fun(42);
+ fun(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly {{..}}
+// CHECK-FIXES: fun(1);{{$}}
+}
+
+char c = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly {{..}}
+// CHECK-FIXES: char c = 1;
+
+float f = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'float';{{..}}
+// CHECK-FIXES: float f = 0;
+
+struct Blah {
+ Blah(int blah) { }
+};
+
+const int &ref = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int &ref = 0;
+
+Blah bla = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: Blah bla = 1;
+
+Blah bla2 = 1;
+
+char c2 = 1;
+char c3 = '0';
+bool b = true;
+
+struct BitFields {
+ unsigned a : 3;
+ unsigned flag : 1;
+};
+
+void testBitFields() {
+ BitFields b;
+
+ b.a = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicitly converting
+// CHECK-FIXES: b.a = 1;
+
+ // Don't warn of bitfields of size 1. Unfortunately we can't just
+ // change type of flag to bool, because some compilers like MSVC doesn't
+ // pack bitfields of different types.
+ b.flag = true;
+
+
+}
Index: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - modernize-bool-to-integer-conversion
+
+modernize-bool-to-integer-conversion
+
+
+This check looks for implicit conversion from bool literals to integer types
+
+.. code-block:: C++
+
+ int a = false;
+ vector v(true); // Makes vector of one element
+
+ // Changes it to
+ int a = 0;
+ vector v(1); // Makes vector of one element
+
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,9 +31,9 @@
google-build-using-namespace
google-explicit-constructor
google-global-names-in-headers
- google-readability-braces-around-statements (redirects to readability-braces-around-statements)
+ google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
- google-readability-function-size (redirects to readability-function-size)
+ google-readability-function-size (redirects to readability-function-size)
google-readability-namespace-comments
google-readability-redundant-smartptr-get
google-readability-todo
@@ -76,6 +76,7 @@
misc-unused-parameters
misc-unused-raii
misc-virtual-near-miss
+ modernize-bool-to-integer-conversion
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. http://reviews.llvm.org/D19105 Here is a diff containing fixes for clang. What I see is that it would be nice to detect bitfields of 1 bit and treet it as bool, so it won't warn it such cases. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. In http://reviews.llvm.org/D18821#399079, @alexfh wrote: > In http://reviews.llvm.org/D18821#399064, @Prazek wrote: > > > In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > > > > > BTW, why is the check in the 'modernize' module? It doesn't seem to make > > > anything more modern. I would guess, the pattern it detects is most > > > likely to result from a programming error. Also, the fix, though it > > > retains the behavior, has a high chance to be incorrect. Can you share > > > the results of running this check on LLVM? At least, how many problems it > > > found and how many times the suggested fix was correct. > > > > > > I'd suggest to move the check to `misc` or maybe it's time to create a > > > separate directory for checks targeting various bug-prone patterns. > > > > > > There were many places. > > > Would be nice, if you could tell the number and provide a list of locations. > Have any of these been fixed since then? No, I didn't know that I can do that - I always heard that I can't format code that I didn't change, so I though the same thing is here. I will try do post review of changes in clang/llvm soon. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
alexfh added a comment. In http://reviews.llvm.org/D18821#399064, @Prazek wrote: > In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > > > BTW, why is the check in the 'modernize' module? It doesn't seem to make > > anything more modern. I would guess, the pattern it detects is most likely > > to result from a programming error. Also, the fix, though it retains the > > behavior, has a high chance to be incorrect. Can you share the results of > > running this check on LLVM? At least, how many problems it found and how > > many times the suggested fix was correct. > > > > I'd suggest to move the check to `misc` or maybe it's time to create a > > separate directory for checks targeting various bug-prone patterns. > > > There were many places. Would be nice, if you could tell the number and provide a list of locations. Have any of these been fixed since then? http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. In http://reviews.llvm.org/D18821#398843, @alexfh wrote: > BTW, why is the check in the 'modernize' module? It doesn't seem to make > anything more modern. I would guess, the pattern it detects is most likely to > result from a programming error. Also, the fix, though it retains the > behavior, has a high chance to be incorrect. Can you share the results of > running this check on LLVM? At least, how many problems it found and how many > times the suggested fix was correct. > > I'd suggest to move the check to `misc` or maybe it's time to create a > separate directory for checks targeting various bug-prone patterns. There were many places. Most of them where when assigning true/false to some member which was int. The other place was inside some functions that were returning only true or false, but the return type of this function was int, so I guess what programmer meant there was to return bool. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
alexfh added a comment. BTW, why is the check in the 'modernize' module? It doesn't seem to make anything more modern. I would guess, the pattern it detects is most likely to result from a programming error. Also, the fix, though it retains the behavior, has a high chance to be incorrect. Can you share the results of running this check on LLVM? At least, how many problems it found and how many times the suggested fix was correct. I'd suggest to move the check to `misc` or maybe it's time to create a separate directory for checks targeting various bug-prone patterns. Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:43 @@ +42,3 @@ + + if (!Location.isMacroID()) +Diag << 0 nit: Since both positive and negative branches are present, I'd prefer to make the condition slightly simple by removing the negation. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek updated this revision to Diff 53114.
Prazek marked 2 inline comments as done.
Prazek added a comment.
Used isMacroID to determinate if it's macro
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
test/clang-tidy/modernize-bool-to-integer-conversion.cpp
Index: test/clang-tidy/modernize-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-bool-to-integer-conversion.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s modernize-bool-to-integer-conversion %t
+
+const int is42Answer = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicitly converting bool literal to 'int'; use integer literal instead [modernize-bool-to-integer-conversion]
+// CHECK-FIXES: const int is42Answer = 1;{{$}}
+
+volatile int noItsNot = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicitly converting bool literal to 'int'; {{..}}
+// CHECK-FIXES: volatile int noItsNot = 0;{{$}}
+int a = 42;
+int az = a;
+
+long long ll = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting bool literal to 'long long';{{..}}
+// CHECK-FIXES: long long ll = 1;{{$}}
+
+void fun(int) {}
+#define ONE true
+
+// No fixup for macros.
+int one = ONE;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'int' inside a macro; use integer literal instead [modernize-bool-to-integer-conversion]
+
+void test() {
+ fun(ONE);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly converting bool{{..}}
+
+ fun(42);
+ fun(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly {{..}}
+// CHECK-FIXES: fun(1);{{$}}
+}
+
+char c = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly {{..}}
+// CHECK-FIXES: char c = 1;
+
+float f = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'float';{{..}}
+// CHECK-FIXES: float f = 0;
+
+struct Blah {
+ Blah(int blah) { }
+};
+
+const int &ref = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int &ref = 0;
+
+Blah bla = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: Blah bla = 1;
+
+Blah bla2 = 1;
+
+char c2 = 1;
+char c3 = '0';
+bool b = true;
Index: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - modernize-bool-to-integer-conversion
+
+modernize-bool-to-integer-conversion
+
+
+This check looks for implicit conversion from bool literals to integer types
+
+.. code-block:: C++
+
+ int a = false;
+ vector v(true); // Makes vector of one element
+
+ // Changes it to
+ int a = 0;
+ vector v(1); // Makes vector of one element
+
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,9 +31,9 @@
google-build-using-namespace
google-explicit-constructor
google-global-names-in-headers
- google-readability-braces-around-statements (redirects to readability-braces-around-statements)
+ google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
- google-readability-function-size (redirects to readability-function-size)
+ google-readability-function-size (redirects to readability-function-size)
google-readability-namespace-comments
google-readability-redundant-smartptr-get
google-readability-todo
@@ -76,6 +76,7 @@
misc-unused-parameters
misc-unused-raii
misc-virtual-near-miss
+ modernize-bool-to-integer-conversion
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Replaces C standard library headers with their C++ alternatives.
+- New `modernize-bool-to-integer-conversion
+ `_ check
+
+ Replaces bool literals which are being implicitly cast to integers with integer literals.
+
- New `modernize-raw-string-literal
`_ check
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
==
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek updated this revision to Diff 53113.
Prazek marked 2 inline comments as done.
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
test/clang-tidy/modernize-bool-to-integer-conversion.cpp
Index: test/clang-tidy/modernize-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-bool-to-integer-conversion.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s modernize-bool-to-integer-conversion %t
+
+const int is42Answer = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicitly converting bool literal to 'int'; use integer literal instead [modernize-bool-to-integer-conversion]
+// CHECK-FIXES: const int is42Answer = 1;{{$}}
+
+volatile int noItsNot = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicitly converting bool literal to 'int'; {{..}}
+// CHECK-FIXES: volatile int noItsNot = 0;{{$}}
+int a = 42;
+int az = a;
+
+long long ll = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting bool literal to 'long long';{{..}}
+// CHECK-FIXES: long long ll = 1;{{$}}
+
+void fun(int) {}
+#define ONE true
+
+// No fixup for macros.
+int one = ONE;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'int' inside a macro; use integer literal instead [modernize-bool-to-integer-conversion]
+
+void test() {
+ fun(ONE);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly converting bool{{..}}
+
+ fun(42);
+ fun(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly {{..}}
+// CHECK-FIXES: fun(1);{{$}}
+}
+
+char c = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly {{..}}
+// CHECK-FIXES: char c = 1;
+
+float f = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'float';{{..}}
+// CHECK-FIXES: float f = 0;
+
+struct Blah {
+ Blah(int blah) { }
+};
+
+const int &ref = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int &ref = 0;
+
+Blah bla = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: Blah bla = 1;
+
+Blah bla2 = 1;
+
+char c2 = 1;
+char c3 = '0';
+bool b = true;
Index: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - modernize-bool-to-integer-conversion
+
+modernize-bool-to-integer-conversion
+
+
+This check looks for implicit conversion from bool literals to integer types
+
+.. code-block:: C++
+
+ int a = false;
+ vector v(true); // Makes vector of one element
+
+ // Changes it to
+ int a = 0;
+ vector v(1); // Makes vector of one element
+
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,9 +31,9 @@
google-build-using-namespace
google-explicit-constructor
google-global-names-in-headers
- google-readability-braces-around-statements (redirects to readability-braces-around-statements)
+ google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
- google-readability-function-size (redirects to readability-function-size)
+ google-readability-function-size (redirects to readability-function-size)
google-readability-namespace-comments
google-readability-redundant-smartptr-get
google-readability-todo
@@ -76,6 +76,7 @@
misc-unused-parameters
misc-unused-raii
misc-virtual-near-miss
+ modernize-bool-to-integer-conversion
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Replaces C standard library headers with their C++ alternatives.
+- New `modernize-bool-to-integer-conversion
+ `_ check
+
+ Replaces bool literals which are being implicitly cast to integers with integer literals.
+
- New `modernize-raw-string-literal
`_ check
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tidy/modernize/Mod
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
On Thu, 7 Apr 2016 15:48:56 +, Alexander Kornienko via cfe-commits said: >Actually, did you think about adding this as a clang diagnostic? > >Richard, what do you think about complaining in Clang about `int i = >true;` kind of code? If you don't mind a lurker interjecting... :) I think that'd be great. I work with an old C++ codebase that started before C++ had 'bool', and so it used 'int' instead. A warning for 'int i = true' would help to convert all those old 'ints' to 'bools'. Cheers, Sean ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
alexfh added inline comments.
Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:47
@@ +46,3 @@
+ const auto Type = Cast->getType().getLocalUnqualifiedType();
+ if (isPreprocessorIndependent(BoolLiteral, Result)) {
+diag(BoolLiteral->getLocation(), "implicitly converting bool literal to "
I'd prefer an alternative with less code duplication:
auto Diag = diag("implicitly converting bool literal to %0%select{|inside a
macro}1; use ...") << Type;
if ()
Diag << 1 << FixItHint::...;
else
Diag << 0;
http://reviews.llvm.org/D18821
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. In http://reviews.llvm.org/D18821#394486, @alexfh wrote: > Actually, did you think about adding this as a clang diagnostic? > > Richard, what do you think about complaining in Clang about `int i = true;` > kind of code? Glad to hear that :) http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
alexfh added a comment. Actually, did you think about adding this as a clang diagnostic? Richard, what do you think about complaining in Clang about `int i = true;` kind of code? Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:49 @@ +48,3 @@ +diag(BoolLiteral->getLocation(), "implicitly converting bool literal to " + "%0. Use integer literal instead") +<< Type s/. Use/; use/ Same below. Comment at: docs/ReleaseNotes.rst:119 @@ +118,3 @@ + + Replaces implicit cast from bool literals to integers with int literals. + The phrase is technically incorrect. The check does not replace `implicit cast from bool literals`. It replaces bool literals (which are being implicitly cast to integers) with integer literals. Comment at: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst:9 @@ +8,3 @@ +.. code-block:: C++ + int a = false; + vector v(true); // Makes vector of one element There should be an empty line after `.. code-block ...`. http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek updated this revision to Diff 52855.
Prazek added a comment.
Added new test cases
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
test/clang-tidy/modernize-bool-to-integer-conversion.cpp
Index: test/clang-tidy/modernize-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-bool-to-integer-conversion.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s modernize-bool-to-integer-conversion %t
+
+const int is42Answer = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicitly converting bool literal to 'int'. Use integer literal instead [modernize-bool-to-integer-conversion]
+// CHECK-FIXES: const int is42Answer = 1;{{$}}
+
+volatile int noItsNot = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicitly converting bool literal to 'int'. {{..}}
+// CHECK-FIXES: volatile int noItsNot = 0;{{$}}
+int a = 42;
+int az = a;
+
+long long ll = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting bool literal to 'long long'.{{..}}
+// CHECK-FIXES: long long ll = 1;{{$}}
+
+void fun(int) {}
+#define ONE true
+
+// No fixup for macros.
+int one = ONE;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'int' inside macro. Use integer literal instead [modernize-bool-to-integer-conversion]
+
+void test() {
+ fun(ONE);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly converting bool{{..}}
+
+ fun(42);
+ fun(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly {{..}}
+// CHECK-FIXES: fun(1);{{$}}
+}
+
+char c = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly {{..}}
+// CHECK-FIXES: char c = 1;
+
+float f = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'float'.{{..}}
+// CHECK-FIXES: float f = 0;
+
+struct Blah {
+ Blah(int blah) { }
+};
+
+const int &ref = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int &ref = 0;
+
+Blah bla = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: Blah bla = 1;
+
+Blah bla2 = 1;
+
+char c2 = 1;
+char c3 = '0';
+bool b = true;
Index: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - modernize-bool-to-integer-conversion
+
+modernize-bool-to-integer-conversion
+
+
+This check looks for implicit conversion from bool literals to integer types
+
+.. code-block:: C++
+ int a = false;
+ vector v(true); // Makes vector of one element
+
+ // Changes it to
+ int a = 0;
+ vector v(1); // Makes vector of one element
+
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,9 +31,9 @@
google-build-using-namespace
google-explicit-constructor
google-global-names-in-headers
- google-readability-braces-around-statements (redirects to readability-braces-around-statements)
+ google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
- google-readability-function-size (redirects to readability-function-size)
+ google-readability-function-size (redirects to readability-function-size)
google-readability-namespace-comments
google-readability-redundant-smartptr-get
google-readability-todo
@@ -76,6 +76,7 @@
misc-unused-parameters
misc-unused-raii
misc-virtual-near-miss
+ modernize-bool-to-integer-conversion
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Replaces C standard library headers with their C++ alternatives.
+- New `modernize-bool-to-integer-conversion
+ `_ check
+
+ Replaces implicit cast from bool literals to integers with int literals.
+
- New `modernize-raw-string-literal
`_ check
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tidy/modernize/ModernizeTidyModul
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek updated this revision to Diff 52851.
Prazek marked an inline comment as done.
http://reviews.llvm.org/D18821
Files:
clang-tidy/modernize/BoolToIntegerConversionCheck.cpp
clang-tidy/modernize/BoolToIntegerConversionCheck.h
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
test/clang-tidy/modernize-bool-to-integer-conversion.cpp
Index: test/clang-tidy/modernize-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-bool-to-integer-conversion.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s modernize-bool-to-integer-conversion %t
+
+const int is42Answer = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicitly converting bool literal to 'int'. Use integer literal instead [modernize-bool-to-integer-conversion]
+// CHECK-FIXES: const int is42Answer = 1;{{$}}
+
+volatile int noItsNot = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: implicitly converting bool literal to 'int'. {{..}}
+// CHECK-FIXES: volatile int noItsNot = 0;{{$}}
+int a = 42;
+int az = a;
+
+long long ll = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting bool literal to 'long long'.{{..}}
+// CHECK-FIXES: long long ll = 1;{{$}}
+
+void fun(int) {}
+#define ONE true
+
+// No fixup for macros.
+int one = ONE;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'int' inside macro. Use integer literal instead [modernize-bool-to-integer-conversion]
+
+void test() {
+ fun(ONE);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly converting bool{{..}}
+
+ fun(42);
+ fun(true);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: implicitly {{..}}
+// CHECK-FIXES: fun(1);{{$}}
+}
+
+char c = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly {{..}}
+// CHECK-FIXES: char c = 1;
+
+float f = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: implicitly converting bool literal to 'float'.{{..}}
+// CHECK-FIXES: float f = 0;
+
+char c2 = 1;
+char c3 = '0';
+bool b = true;
Index: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - modernize-bool-to-integer-conversion
+
+modernize-bool-to-integer-conversion
+
+
+This check looks for implicit conversion from bool literals to integer types
+
+.. code-block:: C++
+ int a = false;
+ vector v(true); // Makes vector of one element
+
+ // Changes it to
+ int a = 0;
+ vector v(1); // Makes vector of one element
+
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -31,9 +31,9 @@
google-build-using-namespace
google-explicit-constructor
google-global-names-in-headers
- google-readability-braces-around-statements (redirects to readability-braces-around-statements)
+ google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
- google-readability-function-size (redirects to readability-function-size)
+ google-readability-function-size (redirects to readability-function-size)
google-readability-namespace-comments
google-readability-redundant-smartptr-get
google-readability-todo
@@ -76,6 +76,7 @@
misc-unused-parameters
misc-unused-raii
misc-virtual-near-miss
+ modernize-bool-to-integer-conversion
modernize-deprecated-headers
modernize-loop-convert
modernize-make-unique
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -113,6 +113,11 @@
Replaces C standard library headers with their C++ alternatives.
+- New `modernize-bool-to-integer-conversion
+ `_ check
+
+ Replaces implicit cast from bool literals to integers with int literals.
+
- New `modernize-raw-string-literal
`_ check
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "BoolToIntegerConversionCheck.h"
#include "DeprecatedHeadersCheck.h"
#include "LoopConvertCheck.h"
#include "MakeUniqueCheck.h"
@@ -32,6 +33,8 @@
class ModernizeModule : public ClangTidy
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
mnbvmar added a comment. This check throws a warning also on the conversion to floats (probably very rare ones): double number = true; Even though this behavior is correct, the code warns about the implicit conversion to **integers**. Comment at: docs/ReleaseNotes.rst:119 @@ +118,3 @@ + + Replaces implicit cast from bool literals to integers to int literals. + **with** int literals http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek marked an inline comment as done. Prazek added a comment. In http://reviews.llvm.org/D18821#393556, @mnbvmar wrote: > This check throws a warning also on the conversion to floats (probably very > rare ones): > > double number = true; > > > Even though this behavior is correct, the code warns about the implicit > conversion to **integers**. That case is extremaly rare. I will see what I can do http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
krystyna added inline comments. Comment at: docs/clang-tidy/checks/modernize-bool-to-integer-conversion.rst:9 @@ +8,3 @@ +.. code-block:: C++ + int a = false + vector v(true); // Makes vector of one element int a = false; http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion
Prazek added a comment. Maybe we should merge it with http://reviews.llvm.org/D18745 and name it 'modernize-wrong-literal-cast'. The other question is, will it be better to move it to readability? http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
