tetsuo-cpp created this revision.
tetsuo-cpp added reviewers: njames93, MaskRay.
tetsuo-cpp added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=45189
This patch is adding support for members to the
`bugprone-bool-pointer-implicit-conversion` check. Apologies if I've done this
in a weird/wrong way, I'm still getting my head around the matching DSL.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D83188
Files:
clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
@@ -70,13 +70,72 @@
*(c) = false; // no-warning
}
+#define CHECK(b) \
+ if (b) { \
+ }
+ CHECK(c)
+
struct {
bool *b;
} d = { SomeFunction() };
- if (d.b)
- (void)*d.b; // no-warning
+ if (d.b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dubious check of 'bool *' against 'nullptr'
+ // CHECK-FIXES: if (*d.b) {
+ }
-#define CHECK(b) if (b) {}
- CHECK(c)
+ if (F() && d.b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: dubious check of 'bool *' against 'nullptr'
+ // CHECK-FIXES: if (F() && *d.b) {
+ }
+
+ // TODO: warn here.
+ if (d.b) {
+ G(d.b);
+ }
+
+#define TESTMACRO2 if (d.b || F())
+
+ TESTMACRO2 {
+ }
+
+ t(d.b);
+
+ if (!d.b) {
+ // no-warning
+ }
+
+ if (d.b) {
+ *d.b = true; // no-warning
+ }
+
+ if (d.b) {
+ d.b[0] = false; // no-warning
+ }
+
+ if (d.b) {
+ SomeOtherFunction(d.b); // no-warning
+ }
+
+ if (d.b) {
+ delete[] d.b; // no-warning
+ }
+
+ if (d.b) {
+ *(d.b) = false; // no-warning
+ }
}
+
+struct H {
+ bool *b;
+ void foo() const {
+ if (b) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dubious check of 'bool *' against 'nullptr'
+ // CHECK-FIXES: if (*b) {
+ }
+
+ if (b) {
+ (void)*b; // no-warning
+ }
+ }
+};
Index: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,33 +20,33 @@
Finder->addMatcher(
traverse(
ast_type_traits::TK_AsIs,
- ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!")))),
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType()))),
- ignoringParenImpCasts(declRefExpr().bind("expr")))),
- hasCastKind(CK_PointerToBoolean)))),
- unless(isInTemplateInstantiation()))
+ ifStmt(
+ hasCondition(findAll(implicitCastExpr(
+ unless(hasParent(unaryOperator(hasOperatorName("!")))),
+ hasSourceExpression(expr(
+ hasType(pointerType(pointee(booleanType()))),
+ ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+ memberExpr().bind("expr"))))),
+ hasCastKind(CK_PointerToBoolean)))),
+ unless(isInTemplateInstantiation()))
.bind("if")),
this);
}
-void BoolPointerImplicitConversionCheck::check(
- const MatchFinder::MatchResult &Result) {
- auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
- auto *Var = Result.Nodes.getNodeAs<DeclRefExpr>("expr");
-
+template <typename RefExpr>
+static void checkImpl(const MatchFinder::MatchResult &Result,
+ const RefExpr *Var, const IfStmt *If,
+ const ast_matchers::internal::Matcher<Expr> &DeclRef,
+ ClangTidyCheck &Check) {
// Ignore macros.
if (Var->getBeginLoc().isMacroID())
return;
- // Only allow variable accesses for now, no function calls or member exprs.
+ // Only allow variable accesses and member exprs for now, no function calls.
// 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)
@@ -64,11 +64,30 @@
.empty())
return;
- diag(Var->getBeginLoc(), "dubious check of 'bool *' against 'nullptr', did "
- "you mean to dereference it?")
+ Check.diag(Var->getBeginLoc(),
+ "dubious check of 'bool *' against 'nullptr', did "
+ "you mean to dereference it?")
<< FixItHint::CreateInsertion(Var->getBeginLoc(), "*");
}
+void BoolPointerImplicitConversionCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
+
+ if (const auto *Var = Result.Nodes.getNodeAs<DeclRefExpr>("expr")) {
+ const Decl *D = Var->getDecl();
+ const auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D))));
+ checkImpl(Result, Var, If, DeclRef, *this);
+ }
+
+ if (const auto *Var = Result.Nodes.getNodeAs<MemberExpr>("expr")) {
+ const Decl *D = Var->getMemberDecl();
+ const auto DeclRef =
+ ignoringParenImpCasts(memberExpr(hasDeclaration(equalsNode(D))));
+ checkImpl(Result, Var, If, DeclRef, *this);
+ }
+}
+
} // namespace bugprone
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
