[cfe-commits] r41258 - in /cfe/trunk: AST/CFG.cpp include/clang/AST/CFG.h

[Ted Kremenek]

On Aug 22, 2007, at 9:34 AM, Chris Lattner wrote:

>
> On Aug 22, 2007, at 8:36 AM, Ted Kremenek wrote:
>
>>
>> On Aug 21, 2007, at 10:43 PM, Chris Lattner wrote:
>>
>>>>
>>>> +      // Backpatch the gotos whose label -> block mappings we  
>>>> didn't know
>>>> +      // when we encountered them.
>>>> +      for (BackpatchBlocksTy::iterator I =  
>>>> BackpatchBlocks.begin(),
>>>> +           E = BackpatchBlocks.end(); I != E; ++I ) {
>>>> +
>>>> +        CFGBlock* B = *I;
>>>> +        GotoStmt* G = cast<GotoStmt>(B->getTerminator());
>>>> +        LabelMapTy::iterator LI = LabelMap.find(G->getLabel());
>>>> +
>>>> +        if (LI == LabelMap.end())
>>>> +          return NULL; // No matching label.  Bad CFG.
>>>
>>> This should be an assert.  The CFG should only be built for valid  
>>> ASTs.
>>
>> The current design thought was to allow the construction of CFGs  
>> for arbitrary subtrees of valid ASTs (that represent a function  
>> body).
>>
>> However, a subtree of an AST may contain gotos that reference  
>> labels in other subtrees of the AST (and thus the subtree is  
>> "invalid" from the perspective of constructing a CFG).  If we allow  
>> CFGs to be constructed from arbitrary subtrees of valid ASTs, then  
>> I don't see how this cannot be a run-time check.
>
> Interesting point.  In that case, instead of 'return NULL' you  
> should have 'continue'.
>
> -Chris

This will result in CFGs where there will be blocks with gotos that do  
not have any successors.  Is this the desired behavior?  Should we  
return a flag or something indicating that the control-flow in the CFG  
is "not complete?"