[PATCH] D21298: [Clang-tidy] delete null check

[Aaron Ballman via cfe-commits]

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