[PATCH] [analyzer][Review request] Improved checker naming in CFG dump.

Jordan Rose jordan_rose at apple.com
Fri Feb 14 09:19:09 PST 2014



================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1741-1742
@@ -1740,3 +1740,4 @@
   if (!Errors.empty()) {
-    static SimpleProgramPointTag Tag("MallocChecker : DeadSymbolsLeak");
+    static SimpleProgramPointTag 
+           Tag(getCheckName().getName() + " : DeadSymbolsLeak");
     N = C.addTransition(C.getState(), C.getPredecessor(), &Tag);
----------------
Jordan Rose wrote:
> Ted Kremenek wrote:
> > This is a small improvement, but it is still boilerplate.  It also doesn't enforce regularity.  It would be great if I could write:
> > 
> >   SimpleProgramPointTag Tag(getCheckName(), "DeadSymbolsLeak")
> > 
> > and then just have the constructor do the string formation.  Going one step further, it would be great if I could just write:
> > 
> >   SimpleProgramPointTag Tag(this, "DeadSymbolsLeak")
> > 
> > since "getCheckName()" can easily be called.  That's much simpler and regular. You then don't have to expose string concatenation to the user, and it allows us to change the internal representation of SimpleProgramPointTag later if we should so choose.
> I agree, with the note that it should be a separate CheckerProgramPointTag class. ProgramPointTag is already virtual.
Actually, well...these are statically constructed, which means it's a bit weird that they will use the package name of a particular Checker instance. I guess that's okay in general, but for checkers like MallocChecker that implement more than one check, we //shouldn't// do this, because the check whose name shows up here the first time may not even be enabled on the next run.


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



More information about the cfe-commits mailing list