[cfe-commits] PATCH: Adds basic support for typeid to CFG

[Ted Kremenek]

Scratch that; leave it as 'addStmt'.  It simplifies things.  It will also allow you to remove the logic in UninitializedValues.cpp that you added in r 129431 (as it won't be necessary).

I think it should still be 'return addStmt()' however.  I noticed that VisitUnaryExprOrTypeTraitExpr doesn't do this (which it should).  I'll fix that case.

BTW, the way to test the CFG is with analyses that depend on the CFG, or printing out the CFG and using FileCheck.  For example, if the subexpression itself contains control-flow (e.g., ?:) an analysis may expose issues with the CFG.

On Apr 13, 2011, at 6:44 PM, Ted Kremenek wrote:

> 
> On Apr 13, 2011, at 1:27 AM, Chandler Carruth wrote:
> 
>> Hello,
>> 
>> This patch appears to add 'typeid' support to the CFG. I wrote it while working on r129431, before remembering that -Wuninitialized uses its own custom CFG builder.
>> 
>> Unfortunately, I'm not certain if the patch adds support in the desired manner (I grouped code with similar code for handling sizeof, etc., rather than with C++-specific bits of the code). Also, I'm not sure how best to write a test for this. A pointer or two in the right direction would help there.
>> <add-typeid-to-cfg.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> Hi Chandler,
> 
> +      if (cast<CXXRecordDecl>(Record->getDecl())->isPolymorphic())
> +        addStmt(E->getExprOperand());
> +  }
> 
> Instead of 'addStmt', I think this only needs to be 'Visit'.  'addStmt' is only needed when we want to force the expression to be listed in the CFGBlock, whereas Visit() causes us to do a recursive walk, adding Stmts to the CFGBlock as necessary. 
> 
> Also, this should probably be:
> 
>  return Visit(E->getExprOperand())
> 
> as 'Block' and the return value of that call are not guaranteed to be the same.  I think leaving 'return Block' at the end is fine.