[clang-tools-extra] r212797 - [clang-tidy] Add a checker for implicit bool conversion of a bool*.

[Benjamin Kramer]

Author: d0k
Date: Fri Jul 11 03:08:47 2014
New Revision: 212797

URL: http://llvm.org/viewvc/llvm-project?rev=212797&view=rev
Log:
[clang-tidy] Add a checker for implicit bool conversion of a bool*.

The goal is to find code like the example below, which is likely a typo
where someone meant to write "if (*b)".
bool *b = SomeFunction();
 if (b) {
   // b never dereferenced
 }

This checker naturally has a relatively high false positive rate so it
applies some heuristics to avoid cases where the pointer is checked for
nullptr before being written.

Differential Revision: http://reviews.llvm.org/D4458

Added:
    clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.cpp
    clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.h
    clang-tools-extra/trunk/test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
    clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp

Added: clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.cpp?rev=212797&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.cpp Fri Jul 11 03:08:47 2014
@@ -0,0 +1,70 @@
+//===--- BoolPointerImplicitConversion.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 "BoolPointerImplicitConversion.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace ast_matchers {
+
+AST_MATCHER(CastExpr, isPointerToBoolean) {
+  return Node.getCastKind() == CK_PointerToBoolean;
+}
+AST_MATCHER(QualType, isBoolean) { return Node->isBooleanType(); }
+
+} // namespace ast_matchers
+
+namespace tidy {
+
+void BoolPointerImplicitConversion::registerMatchers(MatchFinder *Finder) {
+  // Look for ifs that have an implicit bool* to bool conversion in the
+  // condition. Filter negations.
+  Finder->addMatcher(
+      ifStmt(hasCondition(findAll(implicitCastExpr(
+                 allOf(unless(hasParent(unaryOperator(hasOperatorName("!")))),
+                       hasSourceExpression(expr(
+                           hasType(pointerType(pointee(isBoolean()))),
+                           ignoringParenImpCasts(declRefExpr().bind("expr")))),
+                       isPointerToBoolean()))))).bind("if"),
+      this);
+}
+
+void
+BoolPointerImplicitConversion::check(const MatchFinder::MatchResult &Result) {
+  auto *If = Result.Nodes.getStmtAs<IfStmt>("if");
+  auto *Var = Result.Nodes.getStmtAs<DeclRefExpr>("expr");
+
+  // Only allow variable accesses for now, no function calls or member exprs.
+  // Check that we don't dereference the variable anywhere within the if. This
+  // avoids false positives for checks of the pointer for nullptr before it is
+  // dereferenced. If there is a dereferencing operator on this variable don't
+  // emit a diagnostic. Also ignore array subscripts.
+  const Decl *D = Var->getDecl();
+  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D))));
+  if (!match(findAll(
+                 unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))),
+             *If, *Result.Context).empty() ||
+      !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If,
+             *Result.Context).empty() ||
+      // FIXME: We should still warn if the paremater is implicitly converted to
+      // bool.
+      !match(findAll(callExpr(hasAnyArgument(DeclRef))), *If, *Result.Context)
+           .empty() ||
+      !match(findAll(deleteExpr(has(expr(DeclRef)))), *If, *Result.Context)
+           .empty())
+    return;
+
+  diag(Var->getLocStart(), "dubious check of 'bool *' against 'nullptr', did "
+                           "you mean to dereference it?")
+      << FixItHint::CreateInsertion(Var->getLocStart(), "*");
+}
+
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.h?rev=212797&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/BoolPointerImplicitConversion.h Fri Jul 11 03:08:47 2014
@@ -0,0 +1,34 @@
+//===--- BoolPointerImplicitConversion.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_MISC_BOOL_POINTER_IMPLICIT_CONV_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOL_POINTER_IMPLICIT_CONV_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Checks for conditions based on implicit conversion from a bool
+/// pointer to bool e.g.
+/// bool *p;
+/// if (p) {
+///   // Never used in a pointer-specific way.
+/// }
+class BoolPointerImplicitConversion : public ClangTidyCheck {
+public:
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOL_POINTER_IMPLICIT_CONV_H
+

Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=212797&r1=212796&r2=212797&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri Jul 11 03:08:47 2014
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
+  BoolPointerImplicitConversion.cpp
   MiscTidyModule.cpp
   RedundantSmartptrGet.cpp
   UseOverride.cpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=212797&r1=212796&r2=212797&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri Jul 11 03:08:47 2014
@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
+#include "BoolPointerImplicitConversion.h"
 #include "RedundantSmartptrGet.h"
 #include "UseOverride.h"
 
@@ -24,6 +25,9 @@ public:
         "misc-argument-comment",
         new ClangTidyCheckFactory<ArgumentCommentCheck>());
     CheckFactories.addCheckFactory(
+        "misc-bool-pointer-implicit-conversion",
+        new ClangTidyCheckFactory<BoolPointerImplicitConversion>());
+    CheckFactories.addCheckFactory(
         "misc-redundant-smartptr-get",
         new ClangTidyCheckFactory<RedundantSmartptrGet>());
     CheckFactories.addCheckFactory(

Added: clang-tools-extra/trunk/test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp?rev=212797&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp Fri Jul 11 03:08:47 2014
@@ -0,0 +1,86 @@
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-bool-pointer-implicit-conversion %t
+// REQUIRES: shell
+
+bool *SomeFunction();
+void SomeOtherFunction(bool*);
+bool F();
+void G(bool);
+
+
+template <typename T>
+void t(T b) {
+  if (b) {
+  }
+}
+
+void foo() {
+  bool *b = SomeFunction();
+  if (b) {
+// CHECK-MESSAGES: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (*b) {
+  }
+
+  if (F() && b) {
+// CHECK-MESSAGES: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (F() && *b) {
+  }
+
+  // TODO: warn here.
+  if (b) {
+    G(b);
+  }
+
+#define TESTMACRO if (b || F())
+
+  TESTMACRO {
+// CHECK-MESSAGES: dubious check of 'bool *' against 'nullptr'
+// Can't fix this.
+// CHECK-FIXES: #define TESTMACRO if (b || F())
+// CHECK-FIXES: TESTMACRO {
+  }
+
+// CHECK-MESSAGES-NOT: warning:
+
+  t(b);
+
+  if (!b) {
+    // no-warning
+  }
+
+  if (SomeFunction()) {
+    // no-warning
+  }
+
+  bool *c = SomeFunction();
+  if (c) {
+    (void)c;
+    (void)*c; // no-warning
+  }
+
+  if (c) {
+    *c = true; // no-warning
+  }
+
+  if (c) {
+    c[0] = false; // no-warning
+  }
+
+  if (c) {
+    SomeOtherFunction(c); // no-warning
+  }
+
+  if (c) {
+    delete[] c; // no-warning
+  }
+
+  if (c) {
+    *(c) = false; // no-warning
+  }
+
+  struct {
+    bool *b;
+  } d = { SomeFunction() };
+
+  if (d.b)
+    (void)*d.b; // no-warning
+}