[cfe-commits] [PATCH] Utilize the indents in -ast-dump to draw structural information

[Manuel Klimek]

On Wed, Jan 23, 2013 at 12:04 PM, Philip Craig <philipjcraig at gmail.com>wrote:

>
>   Does this still need updating for the comment dumping? (and the color
> patch too)
>
>   I don't like how fiddly the lastChild() handling is, but I can't think
> of any alternative besides buffering the output, and I'm not sure doing
> that would be worth the effort.
>
>   Could you include more context in future patches please.
>
>   Manuel, can Phabricator be changed to display a separator when there are
> missing lines due to limited context?
>

Can you point me at where it doesn't? Also, in general, consider uploading
patches with a higher context (see http://llvm.org/docs/Phabricator.html).

Cheers,
/Manuel


>
>
> ================
> Comment at: lib/AST/ASTDumper.cpp:40
> @@ +39,3 @@
> +
> +    /// Indents - Indents[i] indicates if another child exists at level i.
> +    /// Used by Indent() to print the tree structure.
> ----------------
> Don't put the name at the beginning of the comment.
>
> ================
> Comment at: lib/AST/ASTDumper.cpp:541
> @@ -445,1 +540,3 @@
> +  setMoreChildren(HasDeclContext);
> +  if (HasAttrs) {
>      for (AttrVec::const_iterator I = D->getAttrs().begin(),
> ----------------
> I think this test is some code I forgot to delete. The iterator check in
> the for loop is enough, so no need for this or the hasAttrs() call above.
>
> ================
> Comment at: lib/AST/ASTDumper.cpp:549
> @@ -448,2 +548,3 @@
>    }
>    // Decls within functions are visited by the body
> +  setMoreChildren(false);
> ----------------
> This comment might make more sense if it is moved up to HasDeclContext?
>
> ================
> Comment at: lib/AST/ASTDumper.cpp:626
> @@ -519,3 +625,3 @@
>
> -  if (const FunctionTemplateSpecializationInfo *FTSI =
> -      D->getTemplateSpecializationInfo())
> +  bool oldMoreChildren = hasMoreChildren();
> +  const FunctionTemplateSpecializationInfo *FTSI =
> ----------------
> Use upper case for variables. Elsewhere too.
>
>
> http://llvm-reviews.chandlerc.com/D281
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130123/61b75b19/attachment.html>