[PATCH] D55083: Re-arrange content in FunctionDecl dump

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some nit cleanup.



================
Comment at: lib/AST/ASTDumper.cpp:648
+
+  if (const CXXConstructorDecl *C = dyn_cast<CXXConstructorDecl>(D))
+    for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
----------------
Can switch to `const auto *`


================
Comment at: lib/AST/ASTDumper.cpp:649-651
+    for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+                                                 E = C->init_end();
+         I != E; ++I)
----------------
Might as well switch this to use a range-based for loop over `inits()`


================
Comment at: test/AST/ast-dump-funcs.cpp:59
   // CHECK: CXXMethodDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:28> col:8 f 'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}} <col:10> col:15 'float'
----------------
steveire wrote:
> aaron.ballman wrote:
> > I don't think this belongs at the start of the function -- the parameter information seems far more interesting than what the method overrides.
> Output does not occur in order of 'interesting-ness'.
Except that it kind of does occur in order of interesting-ness -- output occurs in whatever order is going to make the AST most understandable to the user.

My worry here is whether the parameter list will get "hidden" by the list of overrides, but I'm not certain this will be a problem in practice. I suspect there aren't a ton of deep class hierarchies with a lot of overrides for the same method across the entire hierarchy. I did some searching to see if I could find code examples in the wild where this output would be demonstrably worse, but I didn't find any, so I think this may be okay.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083