Author: flovent Date: 2026-06-10T16:15:05+08:00 New Revision: e65b4e7fa233fd15fb1e84a66329b8936a594b2d
URL: https://github.com/llvm/llvm-project/commit/e65b4e7fa233fd15fb1e84a66329b8936a594b2d DIFF: https://github.com/llvm/llvm-project/commit/e65b4e7fa233fd15fb1e84a66329b8936a594b2d.diff LOG: [clang-tidy] Avoid invalid fixes in `readability-delete-null-pointer` (#202488) Only provide warnings (not fixits) when `IfStmt` has condition variable or initializer. Note that i didn't provide fixit for the situation that conditon variable is different with the pointer variable being cast to bool because i think this is rare. (the third newly added testcase) Closes #202312. --------- Co-authored-by: Zeyi Xu <[email protected]> Added: Modified: clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/readability/delete-null-pointer.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp index b7e0d9c236973..f469115594611 100644 --- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp @@ -34,8 +34,14 @@ void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) { const auto BinaryPointerCheckCondition = binaryOperator(hasOperands( anyOf(cxxNullPtrLiteralExpr(), integerLiteral(equals(0))), PointerExpr)); + const auto IfWithScopedCondition = + ifStmt(anyOf(hasInitStatement(stmt()), + hasConditionVariableStatement(declStmt()))) + .bind("ifWithScopedCondition"); + Finder->addMatcher( ifStmt(hasCondition(anyOf(PointerExpr, BinaryPointerCheckCondition)), + optionally(IfWithScopedCondition), hasThen(anyOf( DeleteExpr, DeleteMemberExpr, compoundStmt(anyOf(has(DeleteExpr), has(DeleteMemberExpr)), @@ -47,12 +53,14 @@ void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) { void DeleteNullPointerCheck::check(const MatchFinder::MatchResult &Result) { const auto *IfWithDelete = Result.Nodes.getNodeAs<IfStmt>("ifWithDelete"); + const bool HasScopedCondition = + Result.Nodes.getNodeAs<IfStmt>("ifWithScopedCondition") != nullptr; const auto *Compound = Result.Nodes.getNodeAs<CompoundStmt>("compound"); auto Diag = diag( IfWithDelete->getBeginLoc(), "'if' statement is unnecessary; deleting null pointer has no effect"); - if (IfWithDelete->hasElseStorage()) + if (HasScopedCondition || IfWithDelete->hasElseStorage()) return; // FIXME: generate fixit for this case. diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8e83ef7feb670..322cf9a781384 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -723,6 +723,10 @@ Changes in existing checks - Correctly detecting ``this`` usage when a generic lambda calls an overloaded member function. +- Improved :doc:`readability-delete-null-pointer + <clang-tidy/checks/readability/delete-null-pointer>` check by avoiding invalid + fix-its for ``if`` statements with condition variables or initializers. + - Improved :doc:`readability-else-after-return <clang-tidy/checks/readability/else-after-return>` check: diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/delete-null-pointer.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/delete-null-pointer.cpp index b8221c1e296de..a321b1ca8c6cf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/delete-null-pointer.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/delete-null-pointer.cpp @@ -156,3 +156,23 @@ void g() { delete p6; } } + +struct X {}; +X *makeX(); +struct Y {}; +Y *makeY(); + +void conditionVariable() { + if (X *x = makeX()) + delete x; + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect + + if (X *x = makeX(); x) + delete x; + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect + + X *x = makeX(); + if (Y *y = makeY(); x) + delete x; + // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
