steveire created this revision.
steveire added reviewers: aaron.ballman, njames93.
Herald added subscribers: nullptr.cpp, xazax.hun.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Because it no longer relies on finding implicit casts, this check now
works on templates which are not instantiated in the translation unit.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D96138
Files:
clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-delete-null-pointer.cpp
@@ -37,6 +37,34 @@
ti3.foo();
}
+template <typename T>
+struct NeverInstantiated {
+ void foo() {
+ // t1
+ if (mem) // t2
+ delete mem;
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'if' statement is unnecessary;
+ // CHECK-FIXES: // t1
+ // CHECK-FIXES-NEXT: {{^ }}// t2
+ // CHECK-FIXES-NEXT: delete mem;
+ }
+ T mem;
+};
+
+template <typename T>
+struct NeverInstantiatedPtr {
+ void foo() {
+ // t3
+ if (mem) // t4
+ delete mem;
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 'if' statement is unnecessary;
+ // CHECK-FIXES: // t3
+ // CHECK-FIXES-NEXT: {{^ }}// t4
+ // CHECK-FIXES-NEXT: delete mem;
+ }
+ T *mem;
+};
+
void f() {
int *ps = 0;
if (ps /**/) // #0
Index: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
+++ clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.h
@@ -26,6 +26,9 @@
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
};
} // namespace readability
Index: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -20,34 +20,30 @@
void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) {
const auto DeleteExpr =
- cxxDeleteExpr(has(castExpr(has(declRefExpr(
- to(decl(equalsBoundNode("deletedPointer"))))))))
+ cxxDeleteExpr(
+ has(declRefExpr(to(decl(equalsBoundNode("deletedPointer"))))))
.bind("deleteExpr");
const auto DeleteMemberExpr =
- cxxDeleteExpr(has(castExpr(has(memberExpr(hasDeclaration(
- fieldDecl(equalsBoundNode("deletedMemberPointer"))))))))
+ cxxDeleteExpr(has(memberExpr(hasDeclaration(
+ fieldDecl(equalsBoundNode("deletedMemberPointer"))))))
.bind("deleteMemberExpr");
- const auto PointerExpr = ignoringImpCasts(anyOf(
+ const auto PointerExpr = anyOf(
declRefExpr(to(decl().bind("deletedPointer"))),
- memberExpr(hasDeclaration(fieldDecl().bind("deletedMemberPointer")))));
+ memberExpr(hasDeclaration(fieldDecl().bind("deletedMemberPointer"))));
- const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean),
- hasSourceExpression(PointerExpr));
- const auto BinaryPointerCheckCondition = binaryOperator(
- hasOperands(castExpr(hasCastKind(CK_NullToPointer)), PointerExpr));
+ const auto BinaryPointerCheckCondition = binaryOperator(hasOperands(
+ anyOf(cxxNullPtrLiteralExpr(), integerLiteral(equals(0))), PointerExpr));
Finder->addMatcher(
- traverse(TK_AsIs,
- ifStmt(hasCondition(
- anyOf(PointerCondition, BinaryPointerCheckCondition)),
- hasThen(anyOf(DeleteExpr, DeleteMemberExpr,
- compoundStmt(anyOf(has(DeleteExpr),
- has(DeleteMemberExpr)),
- statementCountIs(1))
- .bind("compound"))))
- .bind("ifWithDelete")),
+ ifStmt(hasCondition(anyOf(PointerExpr, BinaryPointerCheckCondition)),
+ hasThen(anyOf(
+ DeleteExpr, DeleteMemberExpr,
+ compoundStmt(anyOf(has(DeleteExpr), has(DeleteMemberExpr)),
+ statementCountIs(1))
+ .bind("compound"))))
+ .bind("ifWithDelete"),
this);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
