llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Ritish Srivastava (Ritish134)
<details>
<summary>Changes</summary>
The readability-delete-null-pointer check correctly warns when an
`if (ptr) { delete ptr; }` has an else branch, but previously bailed
out without generating a fix-it
This patch implements the fix-it for the else case. When both the
then-body and else-body are compound statements (wrapped in braces),
we strip the `if/else` structure and keep both bodies:
### Before: `if (p) { delete p; } else { doSomething(); }`
### After: `delete p; doSomething();`
This transformation is safe because deleting a null pointer is a no-op
per the `C++ standard` [expr.delete].
---
Full diff: https://github.com/llvm/llvm-project/pull/196783.diff
2 Files Affected:
- (modified)
clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp (+14-3)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/readability/delete-null-pointer.cpp
(+16-1)
``````````diff
diff --git
a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
index b7e0d9c236973..2b01bd1fab5fd 100644
--- a/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
using namespace clang::ast_matchers;
@@ -52,9 +53,6 @@ void DeleteNullPointerCheck::check(const
MatchFinder::MatchResult &Result) {
auto Diag = diag(
IfWithDelete->getBeginLoc(),
"'if' statement is unnecessary; deleting null pointer has no effect");
- if (IfWithDelete->hasElseStorage())
- return;
- // FIXME: generate fixit for this case.
const std::optional<Token> PrevTok = utils::lexer::getPreviousToken(
IfWithDelete->getThen()->getBeginLoc(), *Result.SourceManager,
@@ -65,6 +63,19 @@ void DeleteNullPointerCheck::check(const
MatchFinder::MatchResult &Result) {
Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
IfWithDelete->getBeginLoc(), PrevTok->getLocation()));
+ if (IfWithDelete->hasElseStorage()) {
+ const auto *ElseCompound = dyn_cast<CompoundStmt>(IfWithDelete->getElse());
+ if (Compound && ElseCompound) {
+ Diag << FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(Compound->getLBracLoc()));
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ Compound->getRBracLoc(), ElseCompound->getLBracLoc()));
+ Diag << FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(ElseCompound->getRBracLoc()));
+ }
+ return;
+ }
+
if (Compound) {
Diag << FixItHint::CreateRemoval(
CharSourceRange::getTokenRange(Compound->getLBracLoc()));
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..a24bba63e88fd 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
@@ -125,7 +125,8 @@ void f() {
char *c2;
if (c2) {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
- // CHECK-FIXES: } else {
+ // CHECK-FIXES-NOT: if (c2) {
+ // CHECK-FIXES: delete c2;
// CHECK-FIXES: c2 = c;
delete c2;
} else {
@@ -156,3 +157,17 @@ void g() {
delete p6;
}
}
+
+void h() {
+ int *p7 = nullptr;
+ // #7
+ if (p7) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
+ delete p7;
+ } else {
+ p7 = new int(42);
+ }
+ // CHECK-FIXES: // #7
+ // CHECK-FIXES: delete p7;
+ // CHECK-FIXES: p7 = new int(42);
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/196783
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
