On Mon, Oct 15, 2012 at 8:29 PM, Richard Trieu <[email protected]> wrote:
> 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. Cool, makes sense. Waiting impatiently for Phillip's patch then :) Thanks, /Manuel > > > On Mon, Oct 15, 2012 at 3:02 AM, Manuel Klimek <[email protected]> wrote: > >> Hi Richard, >> >> has there been any progress on this? >> >> Cheers, >> /Manuel >> >> >> On Fri, Aug 31, 2012 at 12:06 AM, Richard Trieu <[email protected]>wrote: >> >>> On Thu, Aug 30, 2012 at 1:06 PM, Alexander Kornienko >>> <[email protected]>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 >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
