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