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

[Manuel Klimek]

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/9721844f/attachment.html>