[PATCH] Expose the name of the checker producing each diagnostic message.

Jordan Rose jordan_rose at apple.com
Mon Feb 3 10:14:04 PST 2014


  This approach seems fine to me. The shared state is a bit ugly, but seems like a reasonable tradeoff here.

  I like "CheckName"; it matches where we want to go a bit better. (Or my impression of where we want to go, anyway.)

  Waiting on Ted for final thumbs-up.


================
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:190
@@ -170,2 +189,3 @@
     CHECKER *checker = new CHECKER();
+    checker->Name = CurrentCheckerName;
     CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
----------------
The one thing I would say about this is that it doesn't make sense for checkers that implement multiple checks. If we really cared, we could make it so that if there's //already// a check name set, we mark the checker as implementing multiple names somehow, and then we can catch cases where such a checker is accidentally used to identify a bug, rather than a specific name. But we don't have to do this now.

================
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:135-136
@@ -133,1 +134,4 @@
 
+// This wrapper is used to ensure, that only StringRefs originating from the
+// CheckerRegistry are used as checker names.
+class CheckerName {
----------------
English nitpick: no comma after "ensure". Also, we should explain //why// this is important: we want to make sure all checker name strings have a lifetime that keeps them alive at least until the path diagnostics have been processed.


http://llvm-reviews.chandlerc.com/D2557



More information about the cfe-commits mailing list