[PATCH] D41538: [analyzer] Fix some checker's output plist not containing the checker name #2

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I'm still not quite sure what's the whole point of having `BugType` without a checker. We can still easily write anything we want in the "category" and "name" fields anyways, so we can easily produce bugs that are indistinguishable to the user from different checkers, while being able to distinguish them when we, as developers, want to figure out what checker throws them, so what was the whole point this whole time? I might be missing something, but if you have time, maybe remove the whole `CheckName`-based constructor and just tie every `BugType` to the checker? The current approach is totally fine though :)



================
Comment at: include/clang/StaticAnalyzer/Core/BugReporter/BugType.h:51
   StringRef getCategory() const { return Category; }
-  StringRef getCheckName() const { return Check.getName(); }
+  StringRef getCheckName() const {
+    // FIXME: This is a workaround to ensure that Check is initialized 
----------------
I suggest:

```
if (Checker)
  return Checker->getCheckname().getName();
return Check.getName();
```

Like, what's the point of storing the StringRef if we can retrieve it any time anyway?

I guess `Checker` does live long enough?


================
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:277
+      BT_leakedvalist.reset(new BugType(
+          CheckNames[ReportUninit ? CK_Uninitialized : CK_Unterminated],
+          "Leaked va_list", categories::MemoryError));
----------------
a.sidorin wrote:
> xazax.hun wrote:
> > a.sidorin wrote:
> > > If I understand correctly, if we report uninitialized and then unterminated, the second report will have wrong CheckName because it is never reset.
> > That is right. Unfortunately, If unterminated check is not turned on but uninitialized is, we can end up with empty check names. I replaced this workaround with a slightly more robust one.
> Maybe we should use different BugTypes for Uninitialized and Unterminated?
Yep, this rings my bells too. We shouldn't race on how do we initialize a `BugType` depending on what bug is encountered first in the translation unit.


https://reviews.llvm.org/D41538