[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D55395#1324699 <https://reviews.llvm.org/D55395#1324699>, @aaron.ballman wrote:

> This is a novel approach that's not used anywhere else in the AST dumper and there are several ways we could handle this, including:
>
> - What's proposed (adding a new node to the tree that's not directly an AST node)
> - Making use of the pointer information. e.g., https://pastebin.com/mh9dHT9L
> - Adding the label before the AST node. e.g., https://pastebin.com/L8YwJTqe
> - Adding the label after the AST node. e.g., https://pastebin.com/gbNahjsd
> - Probably others
>
>   Why this way?
>
>   I'm not a huge fan of adding a new node to the tree that's not an AST node. It expands the tree both vertically (by adding a new node) and horizontally (by indenting everything below that new node) which I find visually distracting for the benefit provided. I personally prefer using the pointer information as it's less structurally disruptive and still provides the same information. I also find it a bit easier to match nodes up that way because the indentation level and tree-like adornments sometimes make it hard for me to determine relationships between when spatially far apart in the tree. There is precedence for using labels + pointers in the AST dumper already -- this is how we handle the prev and parent nodes for declarations, for instance.
>
>   If we're going to invent a novel way to do this, I do not think it should be done in an ad hoc manner, but should instead talk about whether we want to see this done in a more uniform fashion. For instance, how is this information any different than the list of overrides for a method, the previous declaration in a redecl, the parent of an out-of-line function definition, where a default template argument is inherited from, etc (all of which use pointers for relationships)? I don't feel the same way if we go with an existing practice that incrementally improves consistency.


I personally also prefer the first one. However is this really needed in this case ? The first sub-node will always be the combiner,
and then the second node will the the initializer if present. It seems to me that there is no ambiguity here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55395/new/

https://reviews.llvm.org/D55395