[cfe-dev] A patch for printing policy and other stuff

[Ted Kremenek]

On Jun 30, 2009, at 5:01 AM, Olaf Krzikalla wrote:

> Hi @clang,
>
> I've created a patch for some of the issues discussed earlier in "A  
> bunch of more or less related issues" and some other things:
> Into the details:
>
> 1. In ParentMap I added a function "addStmt" which actually adds a  
> Statement or updates the parent relations of an already existing  
> statement. Due to the second functionality I'm rather unsure with  
> the name but updateStmt sounds not right too as it conceals the add  
> part.

Hi Olaf,

Since ParentMap acts as an observer of the parent-child relationships  
in an AST, I'm not so certain about the name 'addStmt' (which seems a  
little content-free).   A couple suggestions for alternate names are  
'updateParents' (or 'updateDescendants'; see comment below),  
'addRootStmt', or 'updateRoot'.  I'm fine with keeping 'addStmt',  
however, as long as the method declaration is well documented (see  
below).

One other nit:

+  void addStmt(Stmt* S);     // adds and/or updates the parent/child- 
relations of the stmt tree of S
+

Please use doxygen style comments that appear before the method name.   
Can you also include in the comment that this isn't a constant-time  
operation (as it is linear in the number of descendants of S)?   
Clients should know that calling this method repetitively could be a  
performance issue if one isn't careful.  Please also indicate in the  
comment that this method only updates the parent-child relationships  
of the descendants of S, not S itself.

Ted