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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
