Hi, Anton. Thanks for looking at this.

The reason CXXNewExpr does not currently have pre- and post- statement checks 
is because it's not clear what a pre-statement check means. By the time the 
analyzer reaches the CXXNewExpr, the constructor for the object has already 
been run (because the CXXConstructExpr is a child expression). This is already 
a problem for custom allocators (operator new). My plan for this is to add a 
new CFG node to represent the allocator call, but I haven't gotten around to 
it. You can see the discussion (from months ago, sadly) in PR12014. There's no 
real reason for it not to have a post-statement callback, though.

What the FIXME was suggesting is not to have individual pre/post checks in the 
cases of the Visit method, but to move all the checks outside the switch 
statement. The reason this has been delayed is because we were supposed to 
switch from passing around ExplodedNodeSets to passing around NodeBuilders, but 
even without that we'd probably still want to be passing in ExplodedNodeSets 
rather than a single ExplodedNode predecessor, and push the loops as far 
inwards as possible.

There are a couple of other statements where we deliberately skipped the pre- 
or post- statement callback. I think ReturnStmt is one, since a post-statement 
check isn't exactly meaningful. But we can probably special-case those just 
before calling out to the CheckerManager.

So with all that said, if you want to perpetuate the current state of affairs 
and add a post-statement callback to CXXNewExpr and a pre-statement callback to 
CXXDeleteExpr, that's all right. (I can guess you're looking at writing some 
kind of new/delete checker, which we could really use.) But if you're going for 
the full refactoring, let's make sure we get it right.

Jordan


On Dec 12, 2012, at 18:37 , Anton Yartsev <[email protected]> wrote:

> Hi all,
> 
> the patch moves all the [Pre/Post]checks from paticular VisitSomething() 
> methods to the common Visit() method (the idea for refactoring came from 
> FIXME notes)
> it also makes the analyzer invoke [Pre/Post]Stmt checkers while processing 
> CXXNewExpr and CXXDeleteExpr (that's what the patch was originally intended 
> for)
> Is it Ok to commit the patch?
> 
> -- 
> Anton
> 
> <movePrePostChecksToVisit.patch>_______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to