[PATCH] D13071: [PATCH] New checker for mismatched operator new/operator delete definitions

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.

================
Comment at: clang-tidy/misc/NewDeleteOverloadsCheck.cpp:66
@@ +65,3 @@
+namespace {
+OverloadedOperatorKind GetCorrespondingOverload(const FunctionDecl *FD) {
+  switch (FD->getOverloadedOperator()) {
----------------
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

> Function names [...] should be camel case, and start with a lower case letter (e.g. openFile() or isFoo()).

================
Comment at: clang-tidy/misc/NewDeleteOverloadsCheck.cpp:168
@@ +167,3 @@
+  SmallVector<const FunctionDecl *, 4> Diagnose;
+  for (const auto *O : Overloads) {
+    const auto &OI = std::find_if(
----------------
Please don't use "O", "l", "I" as variable names.

================
Comment at: clang-tidy/misc/NewDeleteOverloadsCheck.cpp:170
@@ +169,3 @@
+    const auto &OI = std::find_if(
+        Overloads.begin(), Overloads.end(), [&](const FunctionDecl *FD) {
+          if (FD == O)
----------------
I just noticed that this will be an O(N^2) from all new/delete overloads in all classes in a TU. This should probably be not much usually, but I can imagine a corner-case, where this is going to be slooow. How about sharding these by the enclosing record declaration?

================
Comment at: test/clang-tidy/misc-new-delete-overloads-sized-dealloc.cpp:6
@@ +5,3 @@
+struct S {
+  // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: declaration of 'operator delete' has no matching declaration of 'operator new' at the same scope
+  void operator delete(void *ptr, size_t) noexcept; // not a placement delete
----------------
nit: Let's include the check name in brackets to the check pattern once.


http://reviews.llvm.org/D13071