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

[Alexander Kornienko]

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/ ;):

===================================================================
--- 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"?

...
+    // Indicates whether more child are expected at the current tree depth
You mean "more *children*"?

...
@@ -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?

             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 ;)


On Wed, Aug 29, 2012 at 7:11 PM, Richard Trieu <rtrieu at google.com> wrote:

> For too long, -ast-dump would produce drab, monochromatic, textual AST's.
>  Large walls of text to slough through to find the interesting bits.
>  Worse, if a statement filled more than one line, the structural
> information of the tree would be easily lost amid the type names, source
> locations, pointer address, and statement names.  Fear not, a solution
> presents itself!
>
> 1) The addition of ASCII characters into the unused indents at the
> beginning of lines.  Pipes "|", dashes "-", and backslashes "\" quickly
> show relationships between the different statements.  Find siblings,
> parents, and children fast and easy, never lose your place again.
>
> 2) For each statement, different parts are highlighted with different
> colors.  This visually separates the components and allows faster
> differentiating.  Looking for a specific type?  All types are the same
> color.  Same for the statement names.  And source locations.
>
> 3) But wait, there's more!  Since -ast-dump and dump() use the same
> ASTVisitor, you can also get colors in your debugger!  Just use dumpColor()
> instead of dump() and get colors while you debug.
>
> To use, either use "clang -Xclang -ast-dump" or "clang -cc1 -ast-dump
> -fcolor-diagnostics" from a color-enabled terminal.
>
> Implementation detail:
> Color changes were handled by changeColor() and resetColor() from the
> raw_ostream.  The different colors are stored in constants at the top of
> the class before being used in the relevant visit method.
>
> Tree structure printing was handled by a vector of statuses.  The status
> indicates if there's future children at the level of the index.  This is
> enough information to generate the tree structure.  This replaces the
> integer indent counter.
>
> To determine if color printing is required, the dumper queries the
> DiagnosticEngine in the SourceManager.  This is enough for -ast-dump.
>  However, when debugging and call Stmt.dump(), the Stmt doesn't have a
> SourceManager.  It is possible to call dump() with a SourceManager, but to
> simplify things, a dumpColor() was created to force color printing.
>
> I've attached a patch and an image showing the difference between color
> and no color.  Work in progress, comments welcome.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>


-- 
Regards,
Alex
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120830/929dbe09/attachment.html>