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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
