[PATCH] D64274: [analyzer] VirtualCallChecker overhaul.

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Aug 18 15:35:15 PDT 2019

Szelethus accepted this revision.
Szelethus added a comment.

Thank you, and sorry for dragging you through this! At the very least, we got to learn a lot from it :)

Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:93-96
 // This wrapper is used to ensure that only StringRefs originating from the
 // CheckerRegistry are used as check names. We want to make sure all check
 // name strings have a lifetime that keeps them alive at least until the path
 // diagnostics have been processed.
Yea, so this comment isn't incorrect, but it isn't obvious the reason behind this: `CheckerRegistry` gets destructed almost immediately after loading the checkers to `CheckerManager`, the thing we should emphasize here is that those checker names are actually generated as string literals with the inclusion of `Checkers.inc`, which is why their lifetime is long enough (practically infinite).

Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:97
 // diagnostics have been processed.
 class CheckName {
   friend class ::clang::ento::CheckerRegistry;
Grrrr, this has been bugging me for far too long. We should totally rename this to `CheckerName` eventually.

Comment at: clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp:14-20
 const char * const CoreFoundationObjectiveC = "Core Foundation/Objective-C";
 const char * const LogicError = "Logic error";
 const char * const MemoryRefCount =
   "Memory (Core Foundation/Objective-C/OSObject)";
 const char * const MemoryError = "Memory error";
 const char * const UnixAPI = "Unix API";
+const char * const CXXObjectLifecycle = "C++ object lifecycle";
`static constexpr StringLiteral`? Why is there a need to have a header and a cpp file for this? Not that it matters much (definitely not in the context of this patch), but compile-time stuff is cool.



More information about the cfe-commits mailing list