[cfe-commits] [Patch] Visually update -ast-dump

[Richard Trieu]

Phillip Craig was working on a patch for decl dumping and I was waiting for
it to go in so he wouldn't have to deal with the new color and indenting
code I added.

On Mon, Oct 15, 2012 at 3:02 AM, Manuel Klimek <klimek at google.com> wrote:

> Hi Richard,
>
> has there been any progress on this?
>
> Cheers,
> /Manuel
>
>
> On Fri, Aug 31, 2012 at 12:06 AM, Richard Trieu <rtrieu at google.com> wrote:
>
>> On Thu, Aug 30, 2012 at 1:06 PM, Alexander Kornienko <alexfh at google.com>wrote:
>>
>>> Looks nice!
>>>
>>> I'm not the one to approve it, but here are few minor comments regarding
>>> the patch (btw, it would be much more convenient with
>>> http://llvm-reviews.chandlerc.com/
>>>
>> I haven't been paying attention to the review options for this mailing
>> list.  I'll try sending future versions of this patch there.
>>
>>
>>>  ;):
>>>
>>  http://xkcd.com/541/
>>
>>>
>>> ===================================================================
>>> --- lib/AST/StmtDumper.cpp (revision 162862)
>>> +++ lib/AST/StmtDumper.cpp (working copy)
>>> ...
>>> +    static const enum raw_ostream::Colors lineColor =
>>> Are there reasons not to omit "enum" in "enum raw_ostream::Colors"?
>>>
>> It came when I copied the colors from TextDiagnostic.cpp.   I don't think
>> they are necessary in either spot.
>>
>>>
>>> ...
>>> +    // Indicates whether more child are expected at the current tree
>>> depth
>>> You mean "more *children*"?
>>>
>> Yes.
>>
>>>
>>> ...
>>> @@ -62,28 +105,74 @@
>>>            // Print out children.
>>>            Stmt::child_range CI = S->children();
>>>            if (CI) {
>>> +            Indents.push_back(IT_Child);
>>> +            Stmt *TempStmt;
>>> Do you need to declare TempStmt here, and not where it's assigned?
>>>
>> Either way works.
>>
>>>
>>>              while (CI) {
>>>                OS << '\n';
>>> -              DumpSubTree(*CI++);
>>> +              TempStmt = *CI++;
>>> I mean here ^
>>> +              if (!CI)
>>> +                Indents.back() = IT_LastChild;
>>> +              DumpSubTree(TempStmt);
>>>              }
>>> +            Indents.pop_back();
>>>
>>> Besides that, are you going to deal with AST dumping further? I was
>>> planning to start replacing current Decl dumping with something more
>>> informative (based on current -ast-dump-xml implementation, probably) at
>>> some point. It would be nice to try to avoid conflicts ;)
>>>
>>> I did notice the FIXME's in DumpDeclarator().  I'm aware of it, but
>> haven't started any work on it.  I'll ping you if I start working in that
>> direction.  Are you considering starting work on it soon?
>>
>>>
>>>
>>> --
>>> Regards,
>>> Alex
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121015/b73981ff/attachment.html>