Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion
alexfh added a comment.
Sorry for the delay. Your patch was lost in my inbox. Feel free to ping me
earlier.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but
having int as return type.
+If check finds case like this then it function return type to bool.
Prazek wrote:
> alexfh wrote:
> > It may well not be a bug. It's definitely not a bug for `extern "C"`
> > functions and functions in C code. We definitely don't need to change the
> > function return type, since it's a rather involved change (and clang-tidy
> > checks currently are simply not able to make such change correctly in case
> > of a function with external linkage). So please remove this automated fix.
> This check runs only on C++ functions. Maybe checking that the function
> isExternC() would be enough?
As I've said, extern "C" is not the only way to make a function a part of an
API exposed to external users.
Comment at: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp:192
@@ +191,3 @@
+ // CHECK-FIXES: bool yat2() {
+
+ auto l = []() {
Prazek wrote:
> Why it is dangerous? What I see after runnign on llvm code base, that it is
> one of the most frequent case.
> The only bug is the extern "C" thing.
Well, `extern "C"` is not the only way to expose functions to some external
code. This fix potentially changes ABI, which is generally not a safe thing to
do. I think, we should make the fix optional.
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 bugprone-bool-to-integer-conversion
Prazek added inline comments.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but
having int as return type.
+If check finds case like this then it function return type to bool.
alexfh wrote:
> It may well not be a bug. It's definitely not a bug for `extern "C"`
> functions and functions in C code. We definitely don't need to change the
> function return type, since it's a rather involved change (and clang-tidy
> checks currently are simply not able to make such change correctly in case of
> a function with external linkage). So please remove this automated fix.
This check runs only on C++ functions. Maybe checking that the function
isExternC() would be enough?
Comment at: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp:192
@@ +191,3 @@
+ // CHECK-FIXES: bool yat2() {
+
+ auto l = []() {
Why it is dangerous? What I see after runnign on llvm code base, that it is one
of the most frequent case.
The only bug is the extern "C" thing.
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 bugprone-bool-to-integer-conversion
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:37
@@ +36,3 @@
+
+It turns out that the common bug is to have function returning only bools but
having int as return type.
+If check finds case like this then it function return type to bool.
It may well not be a bug. It's definitely not a bug for `extern "C"` functions
and functions in C code. We definitely don't need to change the function return
type, since it's a rather involved change (and clang-tidy checks currently are
simply not able to make such change correctly in case of a function with
external linkage). So please remove this automated fix.
Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:41
@@ +40,3 @@
+
+.. code-block:: C++
+ int fun() {
Please check that your documentation compiles.
Comment at: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp:191
@@ +190,3 @@
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function has return type 'int'
but returns only bools
+ // CHECK-FIXES: bool yat2() {
+
That's a dangerous fix. It should either be removed or guarded by an option.
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 bugprone-bool-to-integer-conversion
Prazek updated this revision to Diff 58961.
Prazek added a comment.
aborting with virtual functions
http://reviews.llvm.org/D18821
Files:
clang-tidy/CMakeLists.txt
clang-tidy/bugprone/BoolToIntegerConversionCheck.cpp
clang-tidy/bugprone/BoolToIntegerConversionCheck.h
clang-tidy/bugprone/BugProneModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/plugin/CMakeLists.txt
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/index.rst
test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
Index: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
@@ -0,0 +1,217 @@
+// RUN: %check_clang_tidy %s bugprone-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 [bugprone-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'; use integer literal instead [bugprone-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 = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int = 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;
+
+// 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.
+struct BitFields {
+ BitFields() : a(true), flag(false) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: implicitly converting
+// CHECK-FIXES: BitFields() : a(1), flag(false) {}
+
+ unsigned a : 3;
+ unsigned flag : 1;
+};
+
+struct some_struct {
+ bool p;
+ int z;
+};
+
+void testBitFields() {
+ BitFields b;
+ b.flag = true;
+ b.a = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicitly converting
+// CHECK-FIXES: b.a = 1;
+
+ b.flag |= true;
+ some_struct s;
+ s.p |= true;
+ s.z |= true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly converting
+// CHECK-FIXES: s.z |= 1;
+}
+
+bool returnsBool() { return true; }
+
+void test_operators() {
+ bool p = false;
+ if (p == false) {
+
+ }
+ int z = 1;
+ if (z == true) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting
+// CHECK-FIXES: if (z == 1) {
+
+ }
+ bool p2 = false != p;
+
+ int z2 = z - true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting
+// CHECK-FIXES: int z2 = z - 1;
+
+ bool p3 = z + true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicitly converting
+// CHECK-FIXES: bool p3 = z + 1;
+
+ if(true && p == false) {}
+ if (returnsBool() == false) {}
+}
+
+bool ok() {
+ return true;
+ {
+return false;
+ }
+ bool z;
+ return z;
+}
+
+int change_type();
+// CHECK-FIXES: bool change_type();
+
+// This function returns only bools
+int change_type() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function has return type 'int' but returns only bools [bugprone-bool-to-integer-conversion]
+ // CHECK-FIXES: bool change_type() {
+
+ bool p;
+ return p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: note: converting bool to 'int' here
+
+ return true;
+ {
+return false;
+ }
+ return ok();
+}
+
+int change_type(int);
+
+char change_type2() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function has return type 'char' but returns only bools [bugprone-bool-to-integer-conversion]
+ // CHECK-FIXES: bool change_type2() {
+
+ bool z;
+ return z;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: note: converting bool to 'char' here
+}
+
Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion
Prazek updated this revision to Diff 58309.
Prazek added a comment.
Some small bugfixes afeter running it on llvm
http://reviews.llvm.org/D18821
Files:
clang-tidy/CMakeLists.txt
clang-tidy/bugprone/BoolToIntegerConversionCheck.cpp
clang-tidy/bugprone/BoolToIntegerConversionCheck.h
clang-tidy/bugprone/BugProneModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/plugin/CMakeLists.txt
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/index.rst
test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
Index: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
@@ -0,0 +1,200 @@
+// RUN: %check_clang_tidy %s bugprone-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 [bugprone-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'; use integer literal instead [bugprone-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 = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int = 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;
+
+// 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.
+struct BitFields {
+ BitFields() : a(true), flag(false) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: implicitly converting
+// CHECK-FIXES: BitFields() : a(1), flag(false) {}
+
+ unsigned a : 3;
+ unsigned flag : 1;
+};
+
+struct some_struct {
+ bool p;
+ int z;
+};
+
+void testBitFields() {
+ BitFields b;
+ b.flag = true;
+ b.a = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicitly converting
+// CHECK-FIXES: b.a = 1;
+
+ b.flag |= true;
+ some_struct s;
+ s.p |= true;
+ s.z |= true;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: implicitly converting
+// CHECK-FIXES: s.z |= 1;
+}
+
+bool returnsBool() { return true; }
+
+void test_operators() {
+ bool p = false;
+ if (p == false) {
+
+ }
+ int z = 1;
+ if (z == true) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting
+// CHECK-FIXES: if (z == 1) {
+
+ }
+ bool p2 = false != p;
+
+ int z2 = z - true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting
+// CHECK-FIXES: int z2 = z - 1;
+
+ bool p3 = z + true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicitly converting
+// CHECK-FIXES: bool p3 = z + 1;
+
+ if(true && p == false) {}
+ if (returnsBool() == false) {}
+}
+
+bool ok() {
+ return true;
+ {
+return false;
+ }
+ bool z;
+ return z;
+}
+
+int change_type();
+// CHECK-FIXES: bool change_type();
+
+// This function returns only bools
+int change_type() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function has return type 'int' but returns only bools [bugprone-bool-to-integer-conversion]
+ // CHECK-FIXES: bool change_type() {
+
+ bool p;
+ return p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: note: converting bool to 'int' here
+
+ return true;
+ {
+return false;
+ }
+ return ok();
+}
+
+int change_type(int);
+
+char change_type2() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function has return type 'char' but returns only bools [bugprone-bool-to-integer-conversion]
+ // CHECK-FIXES: bool change_type2() {
+
+ bool z;
+ return z;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: note: converting bool to 'char'
Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion
Prazek updated this revision to Diff 58148.
Prazek added a comment.
+Fixed bug with operators
+ added fixup for function return type
I will post changes on clang tomorrow
http://reviews.llvm.org/D18821
Files:
clang-tidy/CMakeLists.txt
clang-tidy/bugprone/BoolToIntegerConversionCheck.cpp
clang-tidy/bugprone/BoolToIntegerConversionCheck.h
clang-tidy/bugprone/BugProneModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/plugin/CMakeLists.txt
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/index.rst
test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
Index: test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-bool-to-integer-conversion.cpp
@@ -0,0 +1,163 @@
+// RUN: %check_clang_tidy %s bugprone-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 [bugprone-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'; use integer literal instead [bugprone-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 = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: implicitly converting bool literal to 'int'{{..}}
+// CHECK-FIXES: const int = 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;
+
+// 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.
+struct BitFields {
+ BitFields() : a(true), flag(false) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: implicitly converting
+// CHECK-FIXES: BitFields() : a(1), flag(false) {}
+
+ unsigned a : 3;
+ unsigned flag : 1;
+};
+
+void testBitFields() {
+ BitFields b;
+ b.flag = true;
+ b.a = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicitly converting
+// CHECK-FIXES: b.a = 1;
+}
+
+void test_operators() {
+ bool p = false;
+ if (p == false) {
+
+ }
+ int z = 1;
+ if (z == true) {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting
+// CHECK-FIXES: if (z == 1) {
+
+ }
+ bool p2 = false != p;
+
+ int z2 = z - true;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: implicitly converting
+// CHECK-FIXES: int z2 = z - 1;
+
+ bool p3 = z + true;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: implicitly converting
+// CHECK-FIXES: bool p3 = z + 1;
+}
+
+bool ok() {
+ return true;
+ {
+return false;
+ }
+ bool z;
+ return z;
+}
+
+int change_type();
+// CHECK-FIXES: bool change_type();
+
+// This function returns only bools
+int change_type() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'change_type' has return type 'int' but returns only bools [bugprone-bool-to-integer-conversion]
+ // CHECK-FIXES: bool change_type() {
+
+ bool p;
+ return p;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: note: converting bool to 'int' here
+
+ return true;
+ {
+return false;
+ }
+ return ok();
+}
+
+int change_type(int);
+
+char change_type2() {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'change_type2' has return type 'char' but returns only bools [bugprone-bool-to-integer-conversion]
+ // CHECK-FIXES: bool change_type2() {
+
+ bool z;
+ return z;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: note: converting bool to 'char' here
+}
+
+int return_int() {
+ return 2;
+ {
+return true;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting bool literal
+// CHECK-FIXES: return 1;
+ }
+ bool p;
+ return p;
+}
+
+int
Re: [PATCH] D18821: Add bugprone-bool-to-integer-conversion
alexfh requested changes to this revision. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst:43 @@ +42,3 @@ + +This is because z and ``true`` will be implicitly converted to int by promotion. +To get rid of this case use This is a false positive and should be fixed regardless of whether we disable fixits or not. Just documenting it doesn't help much. 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 bugprone-bool-to-integer-conversion
Prazek added a comment. Updated the diff with changes http://reviews.llvm.org/D19105 http://reviews.llvm.org/D18821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
