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
