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 &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;
+
+// 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 return_int2() {
+ {
+ return true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: implicitly converting bool literal
+ // CHECK-FIXES: return 1;
+ }
+ bool p;
+ return p;
+
+ int z;
+ return z;
+}
Index: docs/clang-tidy/index.rst
===================================================================
--- docs/clang-tidy/index.rst
+++ docs/clang-tidy/index.rst
@@ -67,7 +67,9 @@
* Clang static analyzer checks are named starting with ``clang-analyzer-``.
-* Checks related to Boost library starts with ``boost-``.
+* Checks related to Boost library starts with ``boost-``.
+
+* The ``bugprone-*`` checks target code that have some potential bugs.
Clang diagnostics are treated in a similar way as check diagnostics. Clang
diagnostics are displayed by clang-tidy and can be filtered out using
@@ -347,6 +349,8 @@
* `C++ Core Guidelines
<http://reviews.llvm.org/diffusion/L/browse/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/>`_
+* potential `bugprone code
+ <http://reviews.llvm.org/diffusion/L/browse/clang-tools-extra/trunk/clang-tidy/bugprone/>`_
* `CERT Secure Coding Standards
<http://reviews.llvm.org/diffusion/L/browse/clang-tools-extra/trunk/clang-tidy/cert/>`_
* `Google Style Guide
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
.. toctree::
boost-use-to-string
+ bugprone-bool-to-integer-conversion
cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
cert-dcl50-cpp
cert-dcl54-cpp (redirects to misc-new-delete-overloads) <cert-dcl54-cpp>
Index: docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-bool-to-integer-conversion.rst
@@ -0,0 +1,55 @@
+.. title:: clang-tidy - bugprone-bool-to-integer-conversion
+
+bugprone-bool-to-integer-conversion
+====================================
+
+This check looks for implicit conversion from bool literals to integer types
+
+.. code-block:: C++
+
+ int a = false;
+ vector<bool> v(true); // Makes vector of one element
+ if (a == true) {}
+
+ // Changes it to
+ int a = 0;
+ vector<bool> v(1); // Makes vector of one element
+ if (a == 1) {}
+
+Because bitfields packing are compiler dependent, check treats single-bit
+bitfields as bools
+
+.. code-block:: C++
+
+ struct BitFields {
+ BitFields() : notAFlag(true), b(false) {}
+ unsigned notAFlag : 3;
+ unsigned flag : 1;
+ };
+
+ // Changes to
+ struct BitFields {
+ BitFields() : notAFlag(1), b(false) {}
+ unsigned notAFlag : 3;
+ unsigned flag : 1;
+ };
+
+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.
+
+
+.. code-block:: C++
+ int fun() {
+ return true;
+ return fun();
+ bool z;
+ return z;
+ }
+
+ // changes it to
+ bool fun() {
+ return true;
+ return fun();
+ bool z;
+ return z;
+ }
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -63,6 +63,13 @@
explain them more clearly, and provide more accurate fix-its for the issues
identified. The improvements since the 3.8 release include:
+- New bugprone module containing checks looking for bugprone code.
+
+- New `bugprone-bool-to-integer-conversion
+ <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-bool-to-integer-conversion.html>`_ check
+
+ Replaces bool literals which are being implicitly cast to integers with integer literals.
+
- New Boost module containing checks for issues with Boost library.
- New `boost-use-to-string
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -413,6 +413,11 @@
return 0;
}
+// This anchor is used to force the linker to link the BugProneModule.
+extern volatile int BugProneModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED BugProneModuleAnchorDestination =
+ BugProneModuleAnchorSource;
+
// This anchor is used to force the linker to link the CERTModule.
extern volatile int CERTModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
Index: clang-tidy/tool/CMakeLists.txt
===================================================================
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -10,6 +10,7 @@
clangASTMatchers
clangBasic
clangTidy
+ clangTidyBugProneModule
clangTidyBoostModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
Index: clang-tidy/plugin/CMakeLists.txt
===================================================================
--- clang-tidy/plugin/CMakeLists.txt
+++ clang-tidy/plugin/CMakeLists.txt
@@ -8,6 +8,7 @@
clangFrontend
clangSema
clangTidy
+ clangTidyBugProneModule
clangTidyBoostModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
Index: clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/CMakeLists.txt
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyBugProneModule
+ BugProneModule.cpp
+ BoolToIntegerConversionCheck.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyUtils
+ )
Index: clang-tidy/bugprone/BugProneModule.cpp
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/BugProneModule.cpp
@@ -0,0 +1,40 @@
+//===------- BugProneTidyModule.cpp - clang-tidy --------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "BoolToIntegerConversionCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+class BugProneModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<BoolToIntegerConversionCheck>(
+ "bugprone-bool-to-integer-conversion");
+ }
+};
+
+// Register the BoostModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<BugProneModule> X("bugprone-module",
+ "Add bugprone checks.");
+
+} // namespace bugprone
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the BoostModule.
+volatile int BugProneModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/bugprone/BoolToIntegerConversionCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/BoolToIntegerConversionCheck.h
@@ -0,0 +1,39 @@
+//===--- BoolToIntegerConversionCheck.h - clang-tidy-------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_BOOL_TO_INTEGER_CONVERSION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_BOOL_TO_INTEGER_CONVERSION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds implicit casts of bool literal to integer types like int a = true,
+/// and replaces it with integer literals like int a = 1
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-bool-to-integer-conversion.html
+class BoolToIntegerConversionCheck : public ClangTidyCheck {
+public:
+ BoolToIntegerConversionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void changeFunctionReturnType(const FunctionDecl &FD, const Expr &Return);
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_BOOL_TO_INTEGER_CONVERSION_H
Index: clang-tidy/bugprone/BoolToIntegerConversionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/BoolToIntegerConversionCheck.cpp
@@ -0,0 +1,118 @@
+//===--- BoolToIntegerConversionCheck.cpp - clang-tidy---------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "BoolToIntegerConversionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+AST_MATCHER(FieldDecl, isOneBitBitField) {
+ return Node.isBitField() &&
+ Node.getBitWidthValue(Finder->getASTContext()) == 1;
+}
+
+void BoolToIntegerConversionCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ auto convertingToOneBitBitfieldInInitializer = hasAncestor(
+ cxxConstructorDecl(hasAnyConstructorInitializer(cxxCtorInitializer(
+ forField(isOneBitBitField()),
+ withInitializer(implicitCastExpr(equalsBoundNode("cast")))))));
+
+ auto convertingToOneBitBitfieldByOperator =
+ hasParent(binaryOperator(hasLHS(memberExpr(
+ hasDeclaration(fieldDecl(isOneBitBitField()).bind("bitfield"))))));
+
+ // FIXME: when use "has" matcher instead of hasDescendant when "has" matcher
+ // will not skip casts.
+ auto comparingWithBool = hasParent(binaryOperator(
+ hasDescendant(
+ implicitCastExpr(hasCastKind(CK_LValueToRValue),
+ hasParent(implicitCastExpr().bind("_second_cast")))),
+ hasEitherOperand(implicitCastExpr(hasCastKind(CK_IntegralCast),
+ equalsBoundNode("_second_cast")))));
+
+ auto isInReturnStmt = hasParent(returnStmt());
+ auto returnNotBool = returnStmt(unless(has(expr(hasType(booleanType())))));
+ auto allReturnsBool = hasBody(unless(hasDescendant(returnNotBool)));
+
+ auto soughtCast =
+ implicitCastExpr(has(cxxBoolLiteral().bind("bool_literal"))).bind("cast");
+
+ Finder->addMatcher(
+ stmt(allOf(soughtCast,
+ unless(anyOf(
+ convertingToOneBitBitfieldByOperator,
+ convertingToOneBitBitfieldInInitializer, comparingWithBool,
+ allOf(isInReturnStmt,
+ forFunction(functionDecl(allReturnsBool))))))),
+ this);
+
+ Finder->addMatcher(
+ functionDecl(allReturnsBool,
+ hasDescendant(returnStmt(has(expr().bind("return")))),
+ unless(returns(booleanType())))
+ .bind("function"),
+ this);
+}
+
+void BoolToIntegerConversionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+
+ if (const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function")) {
+ const auto *Return = Result.Nodes.getNodeAs<Expr>("return");
+ return changeFunctionReturnType(*Function, *Return);
+ }
+
+ const auto *BoolLiteral =
+ Result.Nodes.getNodeAs<CXXBoolLiteralExpr>("bool_literal");
+ const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
+ const auto Type = Cast->getType().getLocalUnqualifiedType();
+ auto Location = BoolLiteral->getLocation();
+
+ auto Diag = diag(Location, "implicitly converting bool literal to %0; use "
+ "integer literal instead")
+ << Type;
+
+ if (!Location.isMacroID())
+ Diag << FixItHint::CreateReplacement(BoolLiteral->getSourceRange(),
+ BoolLiteral->getValue() ? "1" : "0");
+}
+
+void BoolToIntegerConversionCheck::changeFunctionReturnType(
+ const FunctionDecl &FD, const Expr &Return) {
+ diag(FD.getLocation(),
+ "function '%0' has return type %1 but returns only bools")
+ << FD.getName() << FD.getReturnType();
+
+ auto Diag = diag(Return.getExprLoc(), "converting bool to %0 here",
+ DiagnosticIDs::Level::Note)
+ << FD.getReturnType();
+
+ // Abort if there is redeclaration in macro
+ for (const auto &Redecl : FD.redecls())
+ if (Redecl->getLocation().isMacroID())
+ return;
+
+ // Change type for all declarations.
+ for (const auto &Redecl : FD.redecls())
+ Diag << FixItHint::CreateReplacement(Redecl->getReturnTypeSourceRange(),
+ "bool");
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/CMakeLists.txt
===================================================================
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -27,6 +27,7 @@
add_subdirectory(tool)
add_subdirectory(plugin)
+add_subdirectory(bugprone)
add_subdirectory(boost)
add_subdirectory(cert)
add_subdirectory(llvm)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
