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

2019-01-15 Thread Stephen Kelly via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC351269: Re-order overrides in FunctionDecl dump (authored by 
steveire, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55083?vs=181744=181892#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D55083

Files:
  lib/AST/ASTDumper.cpp
  test/AST/ast-dump-funcs.cpp


Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -612,20 +612,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (const auto *I : C->inits())
-  dumpCXXCtorInitializer(I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -649,6 +635,22 @@
 }
   }
 
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+ E = C->init_end();
+ I != E; ++I)
+  dumpCXXCtorInitializer(*I);
+
   if (D->doesThisDeclarationHaveABody())
 dumpStmt(D->getBody());
 }
Index: test/AST/ast-dump-funcs.cpp
===
--- test/AST/ast-dump-funcs.cpp
+++ test/AST/ast-dump-funcs.cpp
@@ -56,10 +56,10 @@
 struct T : S { // T is not referenced, but S is
   void f(float, int = 100) override;
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 
'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:21 'int' cinit
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 100
-  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: OverrideAttr
 
   // CHECK: CXXConstructorDecl 0x{{[^ ]*}}  col:8 implicit 
T 'void (const T &)' inline default_delete noexcept-unevaluated


Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -612,20 +612,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (const auto *I : C->inits())
-  dumpCXXCtorInitializer(I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -649,6 +635,22 @@
 }
   }
 
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+ E = C->init_end();
+ I != E; ++I)
+  dumpCXXCtorInitializer(*I);
+
   if (D->doesThisDeclarationHaveABody())
 dumpStmt(D->getBody());
 }
Index: test/AST/ast-dump-funcs.cpp
===
--- test/AST/ast-dump-funcs.cpp
+++ test/AST/ast-dump-funcs.cpp
@@ -56,10 +56,10 @@
 struct T : S { // T is not referenced, but S is
   void f(float, int = 100) override;
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:21 'int' cinit
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 100
-  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void

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

2019-01-15 Thread 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(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{{[^ ]*}}  col:8 f 
'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-01-15 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire marked an inline comment as done.
steveire added inline comments.



Comment at: test/AST/ast-dump-funcs.cpp:59
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 
'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'

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'.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2019-01-15 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire updated this revision to Diff 181744.
steveire added a comment.

Update


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083

Files:
  lib/AST/ASTDumper.cpp
  test/AST/ast-dump-funcs.cpp


Index: test/AST/ast-dump-funcs.cpp
===
--- test/AST/ast-dump-funcs.cpp
+++ test/AST/ast-dump-funcs.cpp
@@ -56,10 +56,10 @@
 struct T : S { // T is not referenced, but S is
   void f(float, int = 100) override;
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 
'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:21 'int' cinit
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 100
-  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: OverrideAttr
 
   // CHECK: CXXConstructorDecl 0x{{[^ ]*}}  col:8 implicit 
T 'void (const T &)' inline default_delete noexcept-unevaluated
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -612,22 +612,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
- E = C->init_end();
- I != E; ++I)
-  dumpCXXCtorInitializer(*I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -651,6 +635,22 @@
 }
   }
 
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+ E = C->init_end();
+ I != E; ++I)
+  dumpCXXCtorInitializer(*I);
+
   if (D->doesThisDeclarationHaveABody())
 dumpStmt(D->getBody());
 }


Index: test/AST/ast-dump-funcs.cpp
===
--- test/AST/ast-dump-funcs.cpp
+++ test/AST/ast-dump-funcs.cpp
@@ -56,10 +56,10 @@
 struct T : S { // T is not referenced, but S is
   void f(float, int = 100) override;
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:21 'int' cinit
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 100
-  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: OverrideAttr
 
   // CHECK: CXXConstructorDecl 0x{{[^ ]*}}  col:8 implicit T 'void (const T &)' inline default_delete noexcept-unevaluated
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -612,22 +612,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
- E = C->init_end();
- I != E; ++I)
-  dumpCXXCtorInitializer(*I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -651,6 +635,22 @@
 }
   }
 
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for

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

2018-12-07 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: test/AST/ast-dump-decl.cpp:217
 // CHECK-NEXT: ParmVarDecl
+// CHECK-NEXT: TemplateArgument
 // CHECK-NEXT: CompoundStmt

I find this ordering to be confusing, especially because the template argument 
information is used by the parameter declaration type.



Comment at: test/AST/ast-dump-funcs.cpp:59
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 
'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'

I don't think this belongs at the start of the function -- the parameter 
information seems far more interesting than what the method overrides.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-12-06 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire updated this revision to Diff 177069.
steveire added a comment.

Adjust tests


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083

Files:
  lib/AST/ASTDumper.cpp
  test/AST/ast-dump-decl.cpp
  test/AST/ast-dump-funcs.cpp
  test/AST/float16.cpp

Index: test/AST/float16.cpp
===
--- test/AST/float16.cpp
+++ test/AST/float16.cpp
@@ -162,8 +162,8 @@
 //CHECK-NEXT: | |   |-DeclRefExpr {{.*}} 'C' lvalue ParmVar {{.*}} 'arg' 'C'
 //CHECK-NEXT: | |   `-FloatingLiteral {{.*}} '_Float16' 2.00e+00
 //CHECK-NEXT: | `-FunctionDecl {{.*}} used func1t '_Float16 (_Float16)'
-//CHECK-NEXT: |   |-TemplateArgument type '_Float16'
 //CHECK-NEXT: |   |-ParmVarDecl {{.*}} used arg '_Float16':'_Float16'
+//CHECK-NEXT: |   |-TemplateArgument type '_Float16'
 //CHECK-NEXT: |   `-CompoundStmt
 //CHECK-NEXT: | `-ReturnStmt
 //CHECK-NEXT: |   `-BinaryOperator {{.*}} '_Float16' '*'
Index: test/AST/ast-dump-funcs.cpp
===
--- test/AST/ast-dump-funcs.cpp
+++ test/AST/ast-dump-funcs.cpp
@@ -56,10 +56,10 @@
 struct T : S { // T is not referenced, but S is
   void f(float, int = 100) override;
   // CHECK: CXXMethodDecl 0x{{[^ ]*}}  col:8 f 'void (float, int)'
+  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:15 'float'
   // CHECK-NEXT: ParmVarDecl 0x{{[^ ]*}}  col:21 'int' cinit
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 100
-  // CHECK-NEXT: Overrides: [ 0x{{[^ ]*}} S::f 'void (float, int)' ]
   // CHECK-NEXT: OverrideAttr
 
   // CHECK: CXXConstructorDecl 0x{{[^ ]*}}  col:8 implicit T 'void (const T &)' inline default_delete noexcept-unevaluated
Index: test/AST/ast-dump-decl.cpp
===
--- test/AST/ast-dump-decl.cpp
+++ test/AST/ast-dump-decl.cpp
@@ -213,20 +213,20 @@
 // CHECK-NEXT: ParmVarDecl{{.*}} 'T'
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate {{.*}}A
-// CHECK-NEXT: TemplateArgument
 // CHECK-NEXT: ParmVarDecl
+// CHECK-NEXT: TemplateArgument
 // CHECK-NEXT: CompoundStmt
 // CHECK-NEXT:   Function{{.*}} 'TestFunctionTemplate' {{.*}}B
 // CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate {{.*}}C
-// CHECK-NEXT: TemplateArgument
 // CHECK-NEXT: ParmVarDecl
-// CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate {{.*}}D
 // CHECK-NEXT: TemplateArgument
+// CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate {{.*}}D
 // CHECK-NEXT: ParmVarDecl
+// CHECK-NEXT: TemplateArgument
 // CHECK-NEXT: CompoundStmt
 // CHECK:  FunctionDecl{{.*}} TestFunctionTemplate {{.*}}B
-// CHECK-NEXT:   TemplateArgument
 // CHECK-NEXT:   ParmVarDecl
+// CHECK-NEXT:   TemplateArgument
 
 namespace testClassTemplateDecl {
   class A { };
@@ -355,8 +355,8 @@
   // CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate 'void (T)'
   // CHECK-NEXT: ParmVarDecl{{.*}} 'T'
   // CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate {{.*}}A
-  // CHECK-NEXT: TemplateArgument
   // CHECK-NEXT: ParmVarDecl
+  // CHECK-NEXT: TemplateArgument
   // CHECK:  FunctionTemplateDecl{{.*}} TestFunctionTemplate
   // CHECK-NEXT:   TemplateTypeParmDecl
   // CHECK-NEXT:   FunctionDecl{{.*}} TestFunctionTemplate 'void (T)'
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -930,22 +930,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
- E = C->init_end();
- I != E; ++I)
-  dumpCXXCtorInitializer(*I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -969,6 +953,22 @@
 }
   }
 
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+

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

2018-11-30 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

In D55083#1314546 , @steveire wrote:

> Yes, please commit your new tests for FunctionDecl dumping before this patch 
> can go in.


r347994 has those tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-11-30 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire added a comment.

Yes, please commit your new tests for FunctionDecl dumping before this patch 
can go in.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-11-30 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

The summary explains what it's doing, but not why it's doing it -- why is this 
change in behavior needed? Does this not break any tests?

Btw, as we work on this refactoring, I think a good approach will be to build 
up the base of tests around AST dumping so that we can be explicitly aware of 
any behavioral changes from the patches. We have some coverage, but it doesn't 
look to be particularly comprehensive. I am happy to contribute some of these 
tests. WDYT?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55083



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2018-11-29 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Output all content which is local to the FunctionDecl before traversing
to child AST nodes.


Repository:
  rC Clang

https://reviews.llvm.org/D55083

Files:
  lib/AST/ASTDumper.cpp


Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -884,22 +884,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
- E = C->init_end();
- I != E; ++I)
-  dumpCXXCtorInitializer(*I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -923,6 +907,22 @@
 }
   }
 
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+ E = C->init_end();
+ I != E; ++I)
+  dumpCXXCtorInitializer(*I);
+
   if (D->doesThisDeclarationHaveABody())
 dumpStmt(D->getBody());
 }


Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -884,22 +884,6 @@
 }
   }
 
-  if (const FunctionTemplateSpecializationInfo *FTSI =
-  D->getTemplateSpecializationInfo())
-dumpTemplateArgumentList(*FTSI->TemplateArguments);
-
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
-  dumpDecl(Parameter);
-
-  if (const CXXConstructorDecl *C = dyn_cast(D))
-for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
- E = C->init_end();
- I != E; ++I)
-  dumpCXXCtorInitializer(*I);
-
   if (const CXXMethodDecl *MD = dyn_cast(D)) {
 if (MD->size_overridden_methods() != 0) {
   auto dumpOverride = [=](const CXXMethodDecl *D) {
@@ -923,6 +907,22 @@
 }
   }
 
+  if (!D->param_begin() && D->getNumParams())
+dumpChild([=] { OS << ">"; });
+  else
+for (const ParmVarDecl *Parameter : D->parameters())
+  dumpDecl(Parameter);
+
+  if (const FunctionTemplateSpecializationInfo *FTSI =
+  D->getTemplateSpecializationInfo())
+dumpTemplateArgumentList(*FTSI->TemplateArguments);
+
+  if (const CXXConstructorDecl *C = dyn_cast(D))
+for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),
+ E = C->init_end();
+ I != E; ++I)
+  dumpCXXCtorInitializer(*I);
+
   if (D->doesThisDeclarationHaveABody())
 dumpStmt(D->getBody());
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits