[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] A patch for printing policy and other stuff
From: Ted Kremenek <kremenek () apple ! com>
Date: 2009-06-30 23:04:44
Message-ID: 7FD9E535-2AD1-422B-9BD3-CA983EAB9660 () apple ! com
[Download RAW message or body]
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
_______________________________________________
cfe-dev mailing list
cfe-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic