[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 reli

[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&id=82761#toc Repository: rL LLVM https://reviews.llvm.org/D

[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 d

[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

[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 Po

[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/

[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

[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

[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(castE

[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/D2129

[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/readabili

[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 unn

[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

[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 redunda

[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

[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/Delet

[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`,

[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; delet

[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/readabil

[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]

[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] --

[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]

[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. Co

[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/DeleteNullPointerChec

[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/Del

[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 w

[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( --

[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/Misc

[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/

[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/DeleteNullPoint

[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

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 o

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().bi

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/D212

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 _