[PATCH] D53543: [analyzer] MallocChecker: pr39348: Realize that sized delete isn't a custom delete.

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> because the guard that prevents it from working is useless and can be removed as well

Should we remove it then?



================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:729
+  SourceLocation L = FD->getLocation();
+  return !L.isValid() || C.getSourceManager().isInSystemHeader(L);
 }
----------------
Should we return true on invalid source location?
Do we have a test case for that?


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2444
-        isStandardNewDelete(FD, Ctx))
-      return;
   }
----------------
why it's fine to remove this branch?


================
Comment at: test/Analysis/NewDelete-custom.cpp:7
 
-#if !(LEAKS && !ALLOCATOR_INLINING)
 // expected-no-diagnostics
 
----------------
MTC wrote:
> Should we continue to keep this line?
yes, since there are not diagnostics emitted. it would complain otherwise.


================
Comment at: test/Analysis/NewDelete-sized-deallocation.cpp:1
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s
+// RUN: %clang_analyze_cc1 -std=c++17 -analyzer-checker=core,cplusplus -verify -analyzer-output=text %s -fsized-deallocation
----------------
MTC wrote:
> I believe `sized deallocation` is the feature since C++14, see https://en.cppreference.com/w/cpp/memory/new/operator_delete. 
specifying 17 is also fine


https://reviews.llvm.org/D53543