[PATCH] D54262: [clang-tidy] Add `delete this` bugprone check (PR38741)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/bugprone/DeleteThisCheck.cpp:23
+      cxxDeleteExpr(
+          has(ignoringParenImpCasts(cxxThisExpr())))
+          .bind("DeleteThis"),
----------------
Will this catch too much? e.g.,
```
struct S {
  int *Foo;

  ~S() { delete this->Foo; }
};
```


================
Comment at: clang-tidy/bugprone/DeleteThisCheck.cpp:30
+  const auto *E = Result.Nodes.getNodeAs<Expr>("DeleteThis");
+  diag(E->getBeginLoc(), "usage of 'delete this' is suspicious");
+}
----------------
Why is it suspicious? It's a valid coding construct that does get used -- looking over LLVM's source, we have a half dozen instances of this construct outside of testing code.

I think this check is likely to have an extremely high false-positive rate unless you add some heuristics to cover common usage patterns that are safe.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54262