DeleteNullPointerCheck now deletes until the end brace of the condition

[J-Camilleri]

Summary

Bug Fix for #35811

[madsravn]

Besides my one comment this looks good to me.

clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp

18

Would you mind creating a new test instead of changing the existing one? Removing tests could create another whole for a bug to fall through.

[J-Camilleri]

Added a new unit test and restored the modified test as it was.

[madsravn]

Looks good to me :)

[J-Camilleri]

In D61861#1500868, @madsravn wrote:

Looks good to me :)

Thanks for the review and suggestion, who should I contact now in order to push the patch?

[madsravn]

In D61861#1500900, @J-Camilleri wrote:

In D61861#1500868, @madsravn wrote:

Looks good to me :)

Thanks for the review and suggestion, who should I contact now in order to push the patch?

I asked around and I was told that we should just leave a comment and then somebody will come around. I was also told that after a few patches you can obtain commit access if you want to push stuff yourself.

someone please commit this.

[madsravn]

This patch has now been merged. https://github.com/llvm/llvm-project/commit/bd324fa2273778430a4fdf8371fec5d64d2231bb

[J-Camilleri]

In D61861#1517812, @madsravn wrote:

This patch has now been merged. https://github.com/llvm/llvm-project/commit/bd324fa2273778430a4fdf8371fec5d64d2231bb

Great, thanks.

clang-tools-extra/clang-tidy/readability/DeleteNullPointerCheck.cpp

 //===--- DeleteNullPointerCheck.cpp - clang-tidy---------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DeleteNullPointerCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
auto Diag = diag(
       IfWithDelete->getBeginLoc(),
       "'if' statement is unnecessary; deleting null pointer has no effect");
   if (IfWithDelete->getElse())
     return;
   // FIXME: generate fixit for this case.
 
   Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
       IfWithDelete->getBeginLoc(),
-      Lexer::getLocForEndOfToken(IfWithDelete->getCond()->getEndLoc(), 0,
+      utils::lexer::getPreviousToken(IfWithDelete->getThen()->getBeginLoc(),
                                      *Result.SourceManager,
-                                 Result.Context->getLangOpts())));
+                                     Result.Context->getLangOpts())
+          .getLocation()));
+
   if (Compound) {
     Diag << FixItHint::CreateRemoval(
         CharSourceRange::getTokenRange(Compound->getLBracLoc()));
     Diag << FixItHint::CreateRemoval(
         CharSourceRange::getTokenRange(Compound->getRBracLoc()));
   }
 }
 
 } // namespace readability
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/readability-delete-null-pointer.cpp

 // RUN: %check_clang_tidy %s readability-delete-null-pointer %t
 
 #define NULL 0
 
 void f() {
+  int *ps = 0;
+  if (ps /**/) // #0
+    delete ps;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+
+  // CHECK-FIXES: int *ps = 0;
+  // CHECK-FIXES-NEXT: {{^  }}// #0
+  // CHECK-FIXES-NEXT: delete ps;
+
   int *p = 0;
 
   // #1
   if (p) { // #2
     delete p;
   } // #3
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
 
   // CHECK-FIXES: {{^  }}// #1
   // CHECK-FIXES-NEXT: {{^  }}// #2
   // CHECK-FIXES-NEXT: delete p;
   // CHECK-FIXES-NEXT: {{^  }}// #3