Index: clang-tidy/misc/BoolPointerImplicitConversion.h =================================================================== --- /dev/null +++ clang-tidy/misc/BoolPointerImplicitConversion.h @@ -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 dereferenced. +/// } +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 + Index: clang-tidy/misc/BoolPointerImplicitConversion.cpp =================================================================== --- /dev/null +++ clang-tidy/misc/BoolPointerImplicitConversion.cpp @@ -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("if"); + auto *Var = Result.Nodes.getStmtAs("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 Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyMiscModule ArgumentCommentCheck.cpp + BoolPointerImplicitConversion.cpp MiscTidyModule.cpp RedundantSmartptrGet.cpp UseOverride.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -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 @@ "misc-argument-comment", new ClangTidyCheckFactory()); CheckFactories.addCheckFactory( + "misc-bool-pointer-implicit-conversion", + new ClangTidyCheckFactory()); + CheckFactories.addCheckFactory( "misc-redundant-smartptr-get", new ClangTidyCheckFactory()); CheckFactories.addCheckFactory( Index: test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp =================================================================== --- /dev/null +++ test/clang-tidy/misc-bool-pointer-implicit-conversion.cpp @@ -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 +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 +}