[llvm-commits] [llvm-gcc-4.2] r93727 - in /llvm-gcc-4.2/trunk/gcc: llvm-convert.cpp llvm-debug.cpp llvm-debug.h llvm-internal.h

[Stuart Hastings]

On Jan 18, 2010, at 1:23 PM, Duncan Sands wrote:

> Hi Stuart,
>
>> +// Starting at the 'desired' BLOCK, recursively walk back to the
>> +// 'grand' context, and return pushing regions to make 'desired' the
>> +// current context.  Assumes 'grand' is a
>> +// parent/grandparent/great-grandparent of 'desired'.  'desired'
>
> how about: "is an ancestor of 'desired'"?

Duh.  Of course.  Agreed; I'll fix.

>
>> +  if (grand != desired)
>> +    push_regions(BLOCK_SUPERCONTEXT(desired), grand);
>
> This shows that this routine can be used with grand == desired, which
> is inconsistent with the comment above about grand being a parent of
> desired (or older).
>
>> +
>> +  void push_regions(tree_node *desired, tree_node *grand);
>
> The comment describing this function is missing here.
>
>> +  void change_regions(tree_node *desired, tree_node *grand);
>
> Likewise.
>
>> +typedef std::set<union tree_node *> treeset;
>
> Here you use "union tree_node *", while...
>
>> +  std::set<tree_node*> seen_blocks;
>
> ... here you do not, which is a bit inconsistent.  Also, why not
> use "treeset" here when declaring seen_blocks?

Generally Agreed to all of the above.  I'll fix.

Thank you,

stuart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100118/f1ce9ee6/attachment.html>