[PATCH] Detect mismatching 'new' and 'delete' uses

[Richard Smith]

This seems like a really nice idea.

================
Comment at: lib/Sema/SemaExprCXX.cpp:2246-2247
@@ +2245,4 @@
+  if (const MemberExpr *ME = dyn_cast<const MemberExpr>(E)) {
+    if (const FieldDecl *FD = dyn_cast<const FieldDecl>(ME->getMemberDecl())) {
+      const CXXRecordDecl *RD = cast<const CXXRecordDecl>(FD->getParent());
+      for (const auto *CD : RD->ctors()) {
----------------
You should probably delay this until the end of the TU, so you can do the check based on knowledge of the full set of constructors (in the common case where there exists a file that defines them all). Take a look at the way we implement the 'unused private field` warning for inspiration.

================
Comment at: lib/Sema/SemaExprCXX.cpp:2249-2250
@@ +2248,4 @@
+      for (const auto *CD : RD->ctors()) {
+        if (NE)
+          break;
+        for (const auto *CI : CD->inits()) {
----------------
I'm not sure that breaking out once you see any `CXXNewExpr` is the right approach. Consider:

  struct S {
    bool Array;
    T *P;
    S() : Array(false), P(new T) {}
    S(int N) : Array(true), P(new T[N]) {}
    ~S() { if (Array) delete [] P; else delete P; }
  };

Here, whether we get a diagnostic, and which one we get, will depend on which constructor we happen to visit first.

================
Comment at: lib/Sema/SemaExprCXX.cpp:2251
@@ +2250,3 @@
+          break;
+        for (const auto *CI : CD->inits()) {
+          if (FD == CI->getMember()) {
----------------
You need to map to the definition of `CD` here.

================
Comment at: lib/Sema/SemaExprCXX.cpp:2253
@@ +2252,3 @@
+          if (FD == CI->getMember()) {
+            NE = dyn_cast<const CXXNewExpr>(CI->getInit());
+            break;
----------------
You should walk over single-element `InitListExpr`s here, for cases like `: p{new T}`, and `IgnoreParenImpCasts`, for cases like `Base *p;` [...] `: p(new Derived)`.

================
Comment at: lib/Sema/SemaExprCXX.cpp:2405
@@ +2404,3 @@
+      Diag(NewLoc, diag::note_allocated_here) << ArrayForm;
+      ArrayForm = !ArrayForm;
+    }
----------------
This is not OK as recovery from a warning, especially not a warning with false positives.

http://reviews.llvm.org/D4661