[PATCH] D21298: [Clang-tidy] delete null check
alexfh added a comment. One more late comment (I should really add a check-list for new checks): this check lacks tests with macros and templates with multiple instantiations. Incorrect handling of templates will likely not manifest in the current state of the check, it's brittle, since it relies on the error deduplication performed by clang-tidy and it can break easily (e.g. if message text will depend on the instantiation or if something changes in the way clang-tidy deduplicates messages). However, attempts to apply fixes to code resulting from macro expansions is unlikely to result in compilable code. Repository: rL LLVM https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
This revision was automatically updated to reflect the committed changes.
Closed by commit rL290784: [clang-tidy] Add delete null pointer check.
(authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D21298?vs=82760=82761#toc
Repository:
rL LLVM
https://reviews.llvm.org/D21298
Files:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-delete-null-pointer.rst
clang-tools-extra/trunk/test/clang-tidy/readability-delete-null-pointer.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Check whether the 'if' statement is unnecessary before calling 'delete' on a pointer.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-delete-null-pointer.html
+class DeleteNullPointerCheck : public ClangTidyCheck {
+public:
+ DeleteNullPointerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -46,6 +47,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
@@ -4,6 +4,7 @@
AvoidConstParamsInDecls.cpp
BracesAroundStatementsCheck.cpp
ContainerSizeEmptyCheck.cpp
+ DeleteNullPointerCheck.cpp
DeletedDefaultCheck.cpp
ElseAfterReturnCheck.cpp
FunctionSizeCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/DeleteNullPointerCheck.cpp
@@ -0,0 +1,76 @@
+//===--- DeleteNullPointerCheck.cpp - clang-tidy---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "DeleteNullPointerCheck.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 {
+namespace readability {
+
+void DeleteNullPointerCheck::registerMatchers(MatchFinder *Finder) {
+ const auto DeleteExpr =
+ cxxDeleteExpr(has(castExpr(has(declRefExpr(
+to(decl(equalsBoundNode("deletedPointer"
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri marked 5 inline comments as done. SilverGeri added inline comments. Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:7 +Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer. +The check is unnecessary as deleting a nullpointer has no effect. + alexfh wrote: > alexfh wrote: > > Either `null pointer` or `nullptr` (enclosed in double backquotes). > Sorry for not being clear enough: "null pointer" is not an inline code > snippet, it shouldn't be enclosed in double backquotes or anything else. The > "(enclosed in double backquotes)" part was meant to apply to `nullptr` only > (since it's a keyword and should be highlighted as a code snippet). To be honest, I don't even understand why I did what I did... :D https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 82760.
SilverGeri added a comment.
reduce number `hasCondition` to 1;
add FIXME comment;
shorten check comments in test
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ 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
+
+ int *p2 = new int[3];
+ // #4
+ if (p2) // #5
+delete[] p2;
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary;
+
+ // CHECK-FIXES: // #4
+ // CHECK-FIXES-NEXT: {{^ }}// #5
+ // CHECK-FIXES-NEXT: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// CHECK-FIXES: } else {
+// CHECK-FIXES: c2 = c;
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the ``if`` statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a null pointer has no effect.
+
+.. code:: c++
+
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define
[PATCH] D21298: [Clang-tidy] delete null check
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
I've noticed a few more minor issues. Otherwise looks good.
Thank you for the new check!
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:27-38
+ const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean));
+ const auto BinaryPointerCheckCondition =
+ binaryOperator(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
+ hasEitherOperand(ignoringImpCasts(declRefExpr(;
+
+ Finder->addMatcher(
+ ifStmt(
This can be simplified. Something like this (modulo formatting):
const auto PointerExpr =
ignoringImpCasts(declRefExpr(to(decl().bind("deletedPointer";
const auto PointerCondition = castExpr(hasCastKind(CK_PointerToBoolean),
hasSourceExpression(PointerExpr));
const auto BinaryPointerCheckCondition =
binaryOperator(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
hasEitherOperand(PointerExpr));
(and remove the second `hasCondition`).
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:53
+ "'if' statement is unnecessary; deleting null pointer has no effect");
+ if (IfWithDelete->getElse())
+return;
The check can suggest a fix in this case as well, but it's a bit more involved.
Please add a FIXME.
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:7
+Checks the 'if' statements where a pointer's existence is checked and then
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
alexfh wrote:
> Either `null pointer` or `nullptr` (enclosed in double backquotes).
Sorry for not being clear enough: "null pointer" is not an inline code snippet,
it shouldn't be enclosed in double backquotes or anything else. The "(enclosed
in double backquotes)" part was meant to apply to `nullptr` only (since it's a
keyword and should be highlighted as a code snippet).
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:8
+
+ // comment that should not be deleted #1
+ if (p) { // comment that should not be deleted #2
The `that should not be deleted` part is superfluous, IMO. You could even leave
just `// #1`, `// #2`, etc.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 82732.
SilverGeri added a comment.
remove redundant `allOf` statements;
refactor test's comment checking part
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ int *p = 0;
+
+ // comment that should not be deleted #1
+ if (p) { // comment that should not be deleted #2
+delete p;
+ } // comment that should not be deleted #3
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+
+ // CHECK-FIXES: {{^ }}// comment that should not be deleted #1
+ // CHECK-FIXES-NEXT: {{^ }}// comment that should not be deleted #2
+ // CHECK-FIXES-NEXT: delete p;
+ // CHECK-FIXES-NEXT: {{^ }}// comment that should not be deleted #3
+
+ int *p2 = new int[3];
+ // comment that should not be deleted #4
+ if (p2) // comment that should not be deleted #5
+delete[] p2;
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: 'if' statement is unnecessary;
+
+ // CHECK-FIXES: // comment that should not be deleted #4
+ // CHECK-FIXES-NEXT: {{^ }}// comment that should not be deleted #5
+ // CHECK-FIXES-NEXT: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// CHECK-FIXES: } else {
+// CHECK-FIXES: c2 = c;
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the ``if`` statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a ``null pointer`` has no effect.
+
+.. code:: c++
+
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+//
[PATCH] D21298: [Clang-tidy] delete null check
alexfh added a comment. In https://reviews.llvm.org/D21298#632265, @alexfh wrote: > In https://reviews.llvm.org/D21298#632235, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote: > > > > > Small ping, is this ready to be committed? > > > > > > If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We > > can deal with any last minute comments post-commit. > > > Yup, that's totally fine, especially when there was a thorough pre-commit > code review. Clarification: it's totally fine to submit without waiting for me longer than a grace period of a day or so, once all comments are addressed and other reviewers have LGTM'd the patch. However, here specifically I have a few more comments ;) https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
alexfh added a comment. In https://reviews.llvm.org/D21298#632235, @aaron.ballman wrote: > In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote: > > > Small ping, is this ready to be committed? > > > If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We can > deal with any last minute comments post-commit. Yup, that's totally fine, especially when there was a thorough pre-commit code review. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:29
+ const auto BinaryPointerCheckCondition = binaryOperator(
+ allOf(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
+hasEitherOperand(ignoringImpCasts(declRefExpr();
Since `binaryOperator` (and most if not all other node matchers) is
`VariadicDynCastAllOfMatcher`, `allOf` is redundant here (similar to Piotr's
comment below).
Same for `compoundStmt` below.
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:6
+
+Checks the 'if' statements where a pointer's existence is checked and then
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
Enclose `if` in double backquotes instead of single quotes.
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:7
+Checks the 'if' statements where a pointer's existence is checked and then
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
Either `null pointer` or `nullptr` (enclosed in double backquotes).
Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:10
+.. code:: c++
+ int *p;
+ if (p)
Insert an empty line above and check that Sphinx can build this.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
This check (also combined with the ones below) is useless. It will work for the
unchanged file as well: it will skip the `if (p)` line and find the comment,
the next CHECK-FIXES will find the `delete p;` line and the CHECK-FIXES-NOT
will find no lines matching `if (p)` between them.
I'd use something like this:
// comment1
if (p) { // comment 2
delete p;
} // comment 3
// CHECK-FIXES: {{^ }}// comment1
// CHECK-FIXES-NEXT: {{^ }} // comment2
// CHECK-FIXES-NEXT: {{^ }} delete p;
// CHECK-FIXES-NEXT: {{^ }} // comment3
Same for patterns below.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
aaron.ballman added a comment. In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote: > Small ping, is this ready to be committed? If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We can deal with any last minute comments post-commit. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
xazax.hun added a comment. Small ping, is this ready to be committed? https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 82401.
SilverGeri added a comment.
remove brackets
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+// comment that should not be deleted
+delete p;
+ }
+ // CHECK-FIXES-NOT: if (p) {
+ // CHECK-FIXES: delete p;
+
+
+ int *p2 = new int[3];
+ // CHECK-FIXES: // another comment to keep
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// another comment to keep
+delete[] p2;
+ // CHECK-FIXES-NOT: if (p2)
+ // CHECK-FIXES: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// CHECK-FIXES: } else {
+// CHECK-FIXES: c2 = c;
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define
[PATCH] D21298: [Clang-tidy] delete null check
aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM, with one nit. You should wait for @alexfh to sign off before committing
though, since he requested changes.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:53
+ "'if' statement is unnecessary; deleting null pointer has no effect");
+ if (IfWithDelete->getElse()) {
+return;
Elide the braces.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 81721.
SilverGeri added a comment.
removing redundant `allOf` from `ifStmt`
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+// comment that should not be deleted
+delete p;
+ }
+ // CHECK-FIXES-NOT: if (p) {
+ // CHECK-FIXES: delete p;
+
+
+ int *p2 = new int[3];
+ // CHECK-FIXES: // another comment to keep
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// another comment to keep
+delete[] p2;
+ // CHECK-FIXES-NOT: if (p2)
+ // CHECK-FIXES: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// CHECK-FIXES: } else {
+// CHECK-FIXES: c2 = c;
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define
[PATCH] D21298: [Clang-tidy] delete null check
Prazek added inline comments. Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:33 + Finder->addMatcher( + ifStmt(allOf(hasCondition( + anyOf(PointerCondition, BinaryPointerCheckCondition)), I think allOf matcher is redundant here because ifStmt takes variadic number of arguments and matches only if all of them matches. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 81458.
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+// comment that should not be deleted
+delete p;
+ }
+ // CHECK-FIXES-NOT: if (p) {
+ // CHECK-FIXES: delete p;
+
+
+ int *p2 = new int[3];
+ // CHECK-FIXES: // another comment to keep
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// another comment to keep
+delete[] p2;
+ // CHECK-FIXES-NOT: if (p2)
+ // CHECK-FIXES: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// CHECK-FIXES: } else {
+// CHECK-FIXES: c2 = c;
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 81452.
SilverGeri added a comment.
remove unused string
using early exit in condition
shorten check-message lines
add check-fisex to 'else' part
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+// comment that should not be deleted
+delete p;
+ }
+ // CHECK-FIXES-NOT: if (p) {
+ // CHECK-FIXES: delete p;
+
+
+ int *p2 = new int[3];
+ // CHECK-FIXES: // another comment to keep
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// another comment to keep
+delete[] p2;
+ // CHECK-FIXES-NOT: if (p2)
+ // CHECK-FIXES: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
+// CHECK-FIXES: } else {
+// CHECK-FIXES: c2 = c;
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef
[PATCH] D21298: [Clang-tidy] delete null check
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:52
+
+ auto D = diag(
+ IfWithDelete->getLocStart(),
Rename `D` to `Diag`, please.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:55
+ "'if' statement is unnecessary; deleting null pointer has no effect");
+ if (!IfWithDelete->getElse()) {
+std::string ReplacementText = Lexer::getSourceText(
hokein wrote:
> I would use an early return `if (IfWithDelete->getElse()) return` here.
Definitely use early exit.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:56
+ if (!IfWithDelete->getElse()) {
+std::string ReplacementText = Lexer::getSourceText(
+CharSourceRange::getTokenRange(
This variable is unused.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:72-76
+ D << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ Compound->getRBracLoc(),
+ Lexer::getLocForEndOfToken(Compound->getRBracLoc(), 0,
+ *Result.SourceManager,
+ Result.Context->getLangOpts(;
Please clang-format the file.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:20
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
+// another comment to keep
Please truncate repeated static parts of the check patterns that exceed 80
characters (e.g. remove the `deleting null pointer has no effect
[readability-delete-null-pointer]` part from all but the first CHECK line).
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:53
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
+delete c2;
+ } else {
Please add CHECK-FIXES lines. Now there's no easy way to see from the test
whether any fixes are applied here.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
hokein accepted this revision.
hokein added a reviewer: hokein.
hokein added a comment.
It looks good to me, but you'd better wait for the approval from @aaron.ballman.
Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:55
+ "'if' statement is unnecessary; deleting null pointer has no effect");
+ if (!IfWithDelete->getElse()) {
+std::string ReplacementText = Lexer::getSourceText(
I would use an early return `if (IfWithDelete->getElse()) return` here.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 79338.
SilverGeri added a comment.
Herald added a subscriber: JDevlieghere.
only warn, not fix when the 'if' statement has 'else' clause
keeping comments
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#define NULL 0
+
+void f() {
+ int *p = 0;
+ // CHECK-FIXES: // comment that should not be deleted
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+// comment that should not be deleted
+delete p;
+ }
+ // CHECK-FIXES-NOT: if (p) {
+ // CHECK-FIXES: delete p;
+
+
+ int *p2 = new int[3];
+ // CHECK-FIXES: // another comment to keep
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+// another comment to keep
+delete[] p2;
+ // CHECK-FIXES-NOT: if (p2)
+ // CHECK-FIXES: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+
+ char *c2;
+ if (c2) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete c2;
+ } else {
+c2 = c;
+ }
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -127,6 +127,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -45,6 +46,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+//
[PATCH] D21298: [Clang-tidy] delete null check
aaron.ballman added inline comments.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
SilverGeri wrote:
> aaron.ballman wrote:
> > hokein wrote:
> > > Does it work the case like:
> > >
> > > ```
> > > int *p = nullptr;
> > > if (p == nullptr) {
> > >p = new int[3];
> > >delete[] p;
> > > }
> > > ```
> > >
> > > ?
> > Similarly, it should not mishandle a case like:
> >
> > void f(int *p) {
> > if (p) {
> > delete p;
> > } else {
> > // Do something else
> > }
> > }
> it warns only if the compund statement contains only one statement (which is
> the delete). We want to warn because it is unnecessary to check the pointer
> validity if you want to just call `delete`. In other cases, we can't be sure
> about the actual behaviour.
In my example, the compound statement does contain only one statement, the
delete, but diagnosing is likely a false positive due to the else clause. In
that case, the pointer validity controls more than just the delete because it
also controls whether to execute the else clause. Removing the if clause
shouldn't be a mechanical change in the presence of an else clause, so the
fixit is definitely inappropriate. I think that diagnosing is still pretty
reasonable, however.
Another test case, which I think may already be handled appropriately but
should still be explicitly tested:
```
if (p) {
// This comment should not disable the check or the fixit.
// Nor should this comment.
delete p;
}
```
I think this check should still be able to remove the if clause, but we should
make sure that the comments don't disable the check, and that the fixit doesn't
destroy the comments.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri added inline comments.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
aaron.ballman wrote:
> hokein wrote:
> > Does it work the case like:
> >
> > ```
> > int *p = nullptr;
> > if (p == nullptr) {
> >p = new int[3];
> >delete[] p;
> > }
> > ```
> >
> > ?
> Similarly, it should not mishandle a case like:
>
> void f(int *p) {
> if (p) {
> delete p;
> } else {
> // Do something else
> }
> }
it warns only if the compund statement contains only one statement (which is
the delete). We want to warn because it is unnecessary to check the pointer
validity if you want to just call `delete`. In other cases, we can't be sure
about the actual behaviour.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
aaron.ballman added inline comments.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
hokein wrote:
> Does it work the case like:
>
> ```
> int *p = nullptr;
> if (p == nullptr) {
>p = new int[3];
>delete[] p;
> }
> ```
>
> ?
Similarly, it should not mishandle a case like:
void f(int *p) {
if (p) {
delete p;
} else {
// Do something else
}
}
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:59
+}
\ No newline at end of file
Please add a newline.
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
hokein added inline comments.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:3
+
+#include
+
We don't rely on implementations of standard headers in lit test. You should
fake the function/class that you need in this test.
Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary;
deleting null pointer has no effect [readability-delete-null-pointer]
Does it work the case like:
```
int *p = nullptr;
if (p == nullptr) {
p = new int[3];
delete[] p;
}
```
?
https://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 77972.
SilverGeri added a comment.
update tests with "CHECK-FIXES-NOT" parts
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,58 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#include
+
+void f() {
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES-NOT: if (p) {
+ // CHECK-FIXES: delete p;
+
+ int *p2 = new int[3];
+ if (p2)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete[] p2;
+ // CHECK-FIXES-NOT: if (p2)
+ // CHECK-FIXES: delete[] p2;
+
+ int *p3 = 0;
+ if (NULL != p3) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p3;
+ }
+ // CHECK-FIXES-NOT: if (NULL != p3) {
+ // CHECK-FIXES: delete p3;
+
+ int *p4 = nullptr;
+ if (p4 != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p4;
+ }
+ // CHECK-FIXES-NOT: if (p4 != nullptr) {
+ // CHECK-FIXES: delete p4;
+
+ char *c;
+ if (c != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete c;
+ }
+ // CHECK-FIXES-NOT: if (c != 0) {
+ // CHECK-FIXES: delete c;
+}
+
+void g() {
+ int *p5, *p6;
+ if (p5)
+delete p6;
+
+ if (p5 && p6)
+delete p5;
+
+ if (p6) {
+int x = 5;
+delete p6;
+ }
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -123,6 +123,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -44,6 +45,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 77784.
SilverGeri added a comment.
Herald added a subscriber: modocache.
move checker to readability module
add missing description to header file
https://reviews.llvm.org/D21298
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/DeleteNullPointerCheck.cpp
clang-tidy/readability/DeleteNullPointerCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-delete-null-pointer.rst
test/clang-tidy/readability-delete-null-pointer.cpp
Index: test/clang-tidy/readability-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-delete-null-pointer.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s readability-delete-null-pointer %t
+
+#include
+
+void f() {
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+ int *p3 = new int[3];
+ if (p3)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete[] p3;
+ // CHECK-FIXES: delete[] p3;
+
+ if (NULL != p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+
+ if (p != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+
+ if (p != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'if' statement is unnecessary; deleting null pointer has no effect [readability-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+}
+
+void g() {
+ int *p, *p2;
+ if (p)
+delete p2;
+
+ if (p && p2)
+delete p;
+
+ if (p2) {
+int x = 5;
+delete p2;
+ }
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/readability-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - readability-delete-null-pointer
+
+readability-delete-null-pointer
+===
+
+Checks the 'if' statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -123,6 +123,7 @@
readability-avoid-const-params-in-decls
readability-braces-around-statements
readability-container-size-empty
+ readability-delete-null-pointer
readability-deleted-default
readability-else-after-return
readability-function-size
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -13,6 +13,7 @@
#include "AvoidConstParamsInDecls.h"
#include "BracesAroundStatementsCheck.h"
#include "ContainerSizeEmptyCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "DeletedDefaultCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionSizeCheck.h"
@@ -44,6 +45,8 @@
"readability-braces-around-statements");
CheckFactories.registerCheck(
"readability-container-size-empty");
+CheckFactories.registerCheck(
+"readability-delete-null-pointer");
CheckFactories.registerCheck(
"readability-deleted-default");
CheckFactories.registerCheck(
Index: clang-tidy/readability/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/readability/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Check whether the 'if' statement is unnecessary before calling 'delete' on a pointer.
+///
+/// For the user-facing documentation see:
[PATCH] D21298: [Clang-tidy] delete null check
Prazek added inline comments. Comment at: test/clang-tidy/misc-delete-null-pointer.cpp:11 + } + // CHECK-FIXES: delete p; + int *p3 = new int[3]; Is there check-fixes-not? This seems to be required here, because even if the fixit won't happen here, the test will pass. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
aaron.ballman added inline comments. Comment at: clang-tidy/misc/DeleteNullPointerCheck.cpp:54 + diag(IfWithDelete->getLocStart(), + "if statement is unnecessary (deleting null pointer has no effect)"); + std::string ReplacementText = Lexer::getSourceText( Rather than a parenthetical, I would just use a semicolon to distinguish the clauses. Also, I would quote 'if' to make it clear that you're talking about syntax rather than an English term. Comment at: clang-tidy/misc/DeleteNullPointerCheck.h:19 + +/// FIXME: Write a short description. +/// You should write that short description. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 77015.
SilverGeri added a comment.
checks `if (p != 0)` conditions, too
https://reviews.llvm.org/D21298
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DeleteNullPointerCheck.cpp
clang-tidy/misc/DeleteNullPointerCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-delete-null-pointer.rst
test/clang-tidy/misc-delete-null-pointer.cpp
Index: test/clang-tidy/misc-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-delete-null-pointer.cpp
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s misc-delete-null-pointer %t
+
+#include
+
+void f() {
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+ int *p3 = new int[3];
+ if (p3)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete[] p3;
+ // CHECK-FIXES: delete[] p3;
+
+ if (NULL != p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+
+ if (p != nullptr) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+
+ if (p != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+}
+
+void g() {
+ int *p, *p2;
+ if (p)
+delete p2;
+
+ if (p && p2)
+delete p;
+
+ if (p2) {
+int x = 5;
+delete p2;
+ }
+}
Index: docs/clang-tidy/checks/misc-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - misc-delete-null-pointer
+
+misc-delete-null-pointer
+
+
+Checks the if statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -58,6 +58,7 @@
misc-bool-pointer-implicit-conversion
misc-dangling-handle
misc-definitions-in-headers
+ misc-delete-null-pointer
misc-fold-init-type
misc-forward-declaration-namespace
misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "MisplacedConstCheck.h"
#include "UnconventionalAssignOperatorCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
@@ -63,6 +64,8 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck(
+"misc-delete-null-pointer");
CheckFactories.registerCheck(
"misc-misplaced-const");
CheckFactories.registerCheck(
Index: clang-tidy/misc/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/misc/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-delete-null-pointer.html
+class DeleteNullPointerCheck : public ClangTidyCheck {
+public:
+ DeleteNullPointerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} //
[PATCH] D21298: [Clang-tidy] delete null check
alexfh added a comment. I guess, "readability" will be a better category for this check. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri updated this revision to Diff 76893.
https://reviews.llvm.org/D21298
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DeleteNullPointerCheck.cpp
clang-tidy/misc/DeleteNullPointerCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-delete-null-pointer.rst
test/clang-tidy/misc-delete-null-pointer.cpp
Index: test/clang-tidy/misc-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-delete-null-pointer.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s misc-delete-null-pointer %t
+
+void f() {
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+ int *p3 = new int[3];
+ if (p3)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete[] p3;
+ // CHECK-FIXES: delete[] p3;
+}
+
+void g() {
+ int *p, *p2;
+ if (p)
+delete p2;
+
+ if (p && p2)
+delete p;
+
+ if (p2) {
+int x = 5;
+delete p2;
+ }
+}
Index: docs/clang-tidy/checks/misc-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - misc-delete-null-pointer
+
+misc-delete-null-pointer
+
+
+Checks the if statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+ int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -58,6 +58,7 @@
misc-bool-pointer-implicit-conversion
misc-dangling-handle
misc-definitions-in-headers
+ misc-delete-null-pointer
misc-fold-init-type
misc-forward-declaration-namespace
misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "MisplacedConstCheck.h"
#include "UnconventionalAssignOperatorCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
@@ -63,6 +64,8 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck(
+"misc-delete-null-pointer");
CheckFactories.registerCheck(
"misc-misplaced-const");
CheckFactories.registerCheck(
Index: clang-tidy/misc/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/misc/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-delete-null-pointer.html
+class DeleteNullPointerCheck : public ClangTidyCheck {
+public:
+ DeleteNullPointerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
Index: clang-tidy/misc/DeleteNullPointerCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/DeleteNullPointerCheck.cpp
@@ -0,0 +1,58 @@
+//===--- DeleteNullPointerCheck.cpp - clang-tidy---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "DeleteNullPointerCheck.h"
[PATCH] D21298: [Clang-tidy] delete null check
SilverGeri removed rL LLVM as the repository for this revision.
SilverGeri updated this revision to Diff 76803.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D21298
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/DeleteNullPointerCheck.cpp
clang-tidy/misc/DeleteNullPointerCheck.h
clang-tidy/misc/MiscTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-delete-null-pointer.rst
test/clang-tidy/misc-delete-null-pointer.cpp
Index: test/clang-tidy/misc-delete-null-pointer.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-delete-null-pointer.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s misc-delete-null-pointer %t
+
+void f() {
+ int *p = 0;
+ if (p) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete p;
+ }
+ // CHECK-FIXES: delete p;
+ int *p3 = new int[3];
+ if (p3)
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: if statement is unnecessary (deleting null pointer has no effect) [misc-delete-null-pointer]
+delete[] p3;
+ // CHECK-FIXES: delete[] p3;
+}
+
+void g() {
+ int *p, *p2;
+ if (p)
+delete p2;
+
+ if (p && p2)
+delete p;
+
+ if (p2) {
+int x = 5;
+delete p2;
+ }
+}
Index: docs/clang-tidy/checks/misc-delete-null-pointer.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-delete-null-pointer.rst
@@ -0,0 +1,12 @@
+.. title:: clang-tidy - misc-delete-null-pointer
+
+misc-delete-null-pointer
+
+
+Checks the if statements where a pointer's existence is checked and then deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+
+.. code:: c++
+int *p;
+ if (p)
+delete p;
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -58,6 +58,7 @@
misc-bool-pointer-implicit-conversion
misc-dangling-handle
misc-definitions-in-headers
+ misc-delete-null-pointer
misc-fold-init-type
misc-forward-declaration-namespace
misc-inaccurate-erase
Index: clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
+#include "DeleteNullPointerCheck.h"
#include "MisplacedConstCheck.h"
#include "UnconventionalAssignOperatorCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
@@ -63,6 +64,8 @@
CheckFactories.registerCheck("misc-argument-comment");
CheckFactories.registerCheck(
"misc-assert-side-effect");
+CheckFactories.registerCheck(
+"misc-delete-null-pointer");
CheckFactories.registerCheck(
"misc-misplaced-const");
CheckFactories.registerCheck(
Index: clang-tidy/misc/DeleteNullPointerCheck.h
===
--- /dev/null
+++ clang-tidy/misc/DeleteNullPointerCheck.h
@@ -0,0 +1,35 @@
+//===--- DeleteNullPointerCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-delete-null-pointer.html
+class DeleteNullPointerCheck : public ClangTidyCheck {
+public:
+ DeleteNullPointerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DELETE_NULL_POINTER_H
Index: clang-tidy/misc/DeleteNullPointerCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/DeleteNullPointerCheck.cpp
@@ -0,0 +1,58 @@
+//===--- DeleteNullPointerCheck.cpp - clang-tidy---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
[PATCH] D21298: [Clang-tidy] delete null check
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/misc-delete-null-pointer.rst:10 +.. code:: c++ +int *p; + if (p) Please indent variable declaration. https://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21298: [Clang-tidy] delete null check
aaron.ballman added inline comments.
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:23
@@ +22,3 @@
+ const auto HasDeleteExpr =
+ cxxDeleteExpr(hasDescendant(declRefExpr().bind("pointerToDelete")))
+ .bind("deleteExpr");
etienneb wrote:
> The use of hasDescendant is Expensive.
> There is cast to ignore? You could probably just skip cast/parens.
>
> If the intend of this match-statement is to match comparison against NULL, it
> should be expanded to be more precise.
I think you want to use `has()` instead of `hasDescendant()` here. Otherwise, I
think this code may trigger on `delete foo(some_declref)` where `foo()` returns
a pointer. You may also need to ignore implicit casts here.
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:35
@@ +34,3 @@
+ .bind("ifWithDelete"),
+ this);
+}
Won't this trigger on code like `if (foo && bar) delete foo->bar;`? It doesn't
look like you handle that sort of case below.
Repository:
rL LLVM
http://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21298: [Clang-tidy] delete null check
etienneb added a comment.
Some comments after a quick look to the code.
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:22
@@ +21,3 @@
+void DeleteNullCheck::registerMatchers(MatchFinder *Finder) {
+ const auto HasDeleteExpr =
+ cxxDeleteExpr(hasDescendant(declRefExpr().bind("pointerToDelete")))
HasDeleteExpr -> DeleteExpr
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:23
@@ +22,3 @@
+ const auto HasDeleteExpr =
+ cxxDeleteExpr(hasDescendant(declRefExpr().bind("pointerToDelete")))
+ .bind("deleteExpr");
The use of hasDescendant is Expensive.
There is cast to ignore? You could probably just skip cast/parens.
If the intend of this match-statement is to match comparison against NULL, it
should be expanded to be more precise.
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:47
@@ +46,3 @@
+
+ if ((CastExpr->getCastKind() == CastKind::CK_PointerToBoolean) &&
+ (PointerToDelete->getDecl() == Cond->getDecl()) &&
This check could be moved to the matcher part above/
see ASTMatcher: hasCastKind
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:48
@@ +47,3 @@
+ if ((CastExpr->getCastKind() == CastKind::CK_PointerToBoolean) &&
+ (PointerToDelete->getDecl() == Cond->getDecl()) &&
+ (!Compound || Compound->size() == 1)) {
see: equalsBoundNode
Comment at: clang-tidy/misc/DeleteNullCheck.cpp:49
@@ +48,3 @@
+ (PointerToDelete->getDecl() == Cond->getDecl()) &&
+ (!Compound || Compound->size() == 1)) {
+auto D = diag(
This check should be moved to the matcher part too.
see:statementCountIs
Repository:
rL LLVM
http://reviews.llvm.org/D21298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21298: [Clang-tidy] delete null check
Eugene.Zelenko added a comment. Will be good idea to change some if statements in regression test to (p != nullptr) (for C++11) and (p != NULL) (pre-C+11). See http://clang.llvm.org/extra/clang-tidy/checks/readability-implicit-bool-cast.html. Repository: rL LLVM http://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21298: [Clang-tidy] delete null check
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Please mention this check in docs/ReleaseNotes.rst (in alphabetical order). I think misc-delete-null-pointer will be better name. Repository: rL LLVM http://reviews.llvm.org/D21298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
