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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
