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

[Richard Smith]

================
Comment at: lib/Sema/SemaExprCXX.cpp:2285
@@ +2284,3 @@
+private:
+  const CXXDeleteExpr *DE;
+  const bool EndOfTU;
----------------
Remove this member and pass `DE` into `analyzeDeleteExpr`, which is the only place that uses it.

================
Comment at: lib/Sema/SemaExprCXX.cpp:2349-2354
@@ +2348,8 @@
+  E = E->IgnoreParenImpCasts();
+  const CXXNewExpr *NE = dyn_cast<const CXXNewExpr>(E);
+  if (!NE) {
+    if (const InitListExpr *ILE = dyn_cast<const InitListExpr>(E))
+      if (ILE->getNumInits() == 1)
+        NE = dyn_cast<const CXXNewExpr>(ILE->getInit(0)->IgnoreParenImpCasts());
+  }
+  return NE;
----------------
Reverse the order of these checks to remove the redundancy:

  if (isa<ILE>(E) && 1 init)
    E = ILE->getInit(0)->IgnoreParenImpCasts()
  auto *NE = dyn_cast<CXXNewExpr>(E);

================
Comment at: lib/Sema/SemaExprCXX.cpp:2371-2372
@@ +2370,4 @@
+
+bool
+MismatchingNewDeleteDetector::analyzeConstructor(const CXXConstructorDecl *CD) {
+  if (CD->isImplicit())
----------------
Please give this a better name, saying what you're actually analyzing and what the return value might mean. Suggestion: switch from returning `false` if you find a matching *new-expression* to returning `true` and rename this to `hasMatchingNewInCtor` (and rename `analyzeCtorInitializer` to `hasMatchingNewInCtorInit`).

================
Comment at: lib/Sema/SemaExprCXX.cpp:2389-2403
@@ +2388,17 @@
+MismatchingNewDeleteDetector::analyzeInClassInitializer() {
+  assert(Field != nullptr && "This should be called only for members");
+  if (HasUndefinedConstructors)
+    return EndOfTU ? NoMismatch : AnalyzeLater;
+  if (!NewExprs.empty())
+    return MemberInitMismatches;
+  if (!Field->hasInClassInitializer())
+    return NoMismatch;
+  if (const CXXNewExpr *NE =
+          getNewExprFromInitListOrExpr(Field->getInClassInitializer())) {
+    if (NE->isArray() != IsArrayForm) {
+      NewExprs.push_back(NE);
+      return MemberInitMismatches;
+    }
+  }
+  return NoMismatch;
+}
----------------
The logic in here is half dealing with in-class initializers and half finalizing the result of your analysis for the field. It would be better to split this up so that this function doesn't do things that are unrelated to in-class initializers.

================
Comment at: lib/Serialization/ASTReader.cpp:3208
@@ +3207,3 @@
+        DelayedDeleteExprs.push_back(getGlobalDeclID(F, Record[I++]));
+        const uint64_t Count = ReadAPInt(Record, I).getZExtValue();
+        DelayedDeleteExprs.push_back(Count);
----------------
Use `const uint64_t Count = Record[I++];` here.

================
Comment at: lib/Serialization/ASTWriter.cpp:4347
@@ +4346,3 @@
+    AddDeclRef(DeleteExprsInfo.first, DeleteExprsToAnalyze);
+    AddAPInt(APInt(8, DeleteExprsInfo.second.size()), DeleteExprsToAnalyze);
+    for (const auto &DeleteLoc : DeleteExprsInfo.second) {
----------------
`APInt` is overkill here. Just use `DeleteExprsToAnalyze.push_back(DeleteExprsInfo.second.size())`.

http://reviews.llvm.org/D4661