[cfe-dev] Check virtual calls in ctor or dtor
kremenek at apple.com
Tue Jan 3 17:27:11 CST 2012
Thanks for your patience Lei. After a break, I was able to get back to reviewing this properly.
Overall, I think this is a great initial version of the checker. We will still want to refine the diagnostics, but I think the base functionality is good. I went ahead and checked in this patch (with changes) in r147494. The overall checker looked good that I thought only a few tweaks were necessary to get it into mainline, so I went and made the changes myself.
Here are the highlights of my changes:
(1) I had Enqueue() do all the checking to see if the method was prescanned or elible for scanning. No need to duplicate that checking in a bunch of places. I added a "NotVisited" value to the enum (which gets default constructed) to avoid uses of "count()" in the DenseMap. This should be slightly more efficient.
(2) For fully resolved calls, e.g. B::foo(), your logic had the checker skipping those calls entirely. That's not quite right. They should be scanned like other calls, just not warned about being virtual calls. I've gone ahead and fixed that.
(3) I doxygenified more of the comments, and cleaned up a bit of the formatting.
I think the next step is to improve the bug reporting. I think the natural way to do this is to provide a way to issue a BugReport with a collection of PathDiagnostics. This allows one to generate a bug report without the assistance of BugReporter constructing it for you.
On Dec 25, 2011, at 2:11 AM, 章磊 wrote:
> Hi Ted,
> I re-implement this checker as you suggested.
> Based on your suggestion, I modified two minor places for the bug
> report generation.
> 1. Use CallExpr instead of FunctionDecl as the WorkList unit.
> 2. Introduced a pointer point to the the CallExpr whose body is
> current walking.
> I test this patch on some real code base, and it seems to work well.
> More comments inline, and looking forward to your reply.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-dev