[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

[Alexander Shaposhnikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308678: [clang-tools-extra] Add support for plain C structs 
in clang-reorder-fields (authored by alexshap).

Changed prior to commit:
  https://reviews.llvm.org/D35329?vs=107143&id=107588#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35329

Files:
  clang-tools-extra/trunk/clang-reorder-fields/ReorderFieldsAction.cpp
  clang-tools-extra/trunk/test/clang-reorder-fields/PlainCStructFieldsOrder.c

Index: clang-tools-extra/trunk/clang-reorder-fields/ReorderFieldsAction.cpp
===
--- clang-tools-extra/trunk/clang-reorder-fields/ReorderFieldsAction.cpp
+++ clang-tools-extra/trunk/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -32,10 +32,10 @@
 /// \brief Finds the definition of a record by name.
 ///
 /// \returns nullptr if the name is ambiguous or not found.
-static const CXXRecordDecl *findDefinition(StringRef RecordName,
-   ASTContext &Context) {
+static const RecordDecl *findDefinition(StringRef RecordName,
+ASTContext &Context) {
   auto Results = match(
-  recordDecl(hasName(RecordName), isDefinition()).bind("cxxRecordDecl"),
+  recordDecl(hasName(RecordName), isDefinition()).bind("recordDecl"),
   Context);
   if (Results.empty()) {
 llvm::errs() << "Definition of " << RecordName << "  not found\n";
@@ -46,14 +46,14 @@
  << " is ambiguous, several definitions found\n";
 return nullptr;
   }
-  return selectFirst("cxxRecordDecl", Results);
+  return selectFirst("recordDecl", Results);
 }
 
 /// \brief Calculates the new order of fields.
 ///
 /// \returns empty vector if the list of fields doesn't match the definition.
 static SmallVector
-getNewFieldsOrder(const CXXRecordDecl *Definition,
+getNewFieldsOrder(const RecordDecl *Definition,
   ArrayRef DesiredFieldsOrder) {
   assert(Definition && "Definition is null");
 
@@ -97,7 +97,7 @@
 /// different accesses (public/protected/private) is not supported.
 /// \returns true on success.
 static bool reorderFieldsInDefinition(
-const CXXRecordDecl *Definition, ArrayRef NewFieldsOrder,
+const RecordDecl *Definition, ArrayRef NewFieldsOrder,
 const ASTContext &Context,
 std::map &Replacements) {
   assert(Definition && "Definition is null");
@@ -223,25 +223,30 @@
   ReorderingConsumer &operator=(const ReorderingConsumer &) = delete;
 
   void HandleTranslationUnit(ASTContext &Context) override {
-const CXXRecordDecl *RD = findDefinition(RecordName, Context);
+const RecordDecl *RD = findDefinition(RecordName, Context);
 if (!RD)
   return;
 SmallVector NewFieldsOrder =
 getNewFieldsOrder(RD, DesiredFieldsOrder);
 if (NewFieldsOrder.empty())
   return;
 if (!reorderFieldsInDefinition(RD, NewFieldsOrder, Context, Replacements))
   return;
-for (const auto *C : RD->ctors())
-  if (const auto *D = dyn_cast(C->getDefinition()))
-reorderFieldsInConstructor(cast(D),
-   NewFieldsOrder, Context, Replacements);
 
-// We only need to reorder init list expressions for aggregate types.
+// CXXRD will be nullptr if C code (not C++) is being processed.
+const CXXRecordDecl *CXXRD = dyn_cast(RD);
+if (CXXRD)
+  for (const auto *C : CXXRD->ctors())
+if (const auto *D = dyn_cast(C->getDefinition()))
+  reorderFieldsInConstructor(cast(D),
+  NewFieldsOrder, Context, Replacements);
+
+// We only need to reorder init list expressions for 
+// plain C structs or C++ aggregate types.
 // For other types the order of constructor parameters is used,
 // which we don't change at the moment.
 // Now (v0) partial initialization is not supported.
-if (RD->isAggregate())
+if (!CXXRD || CXXRD->isAggregate())
   for (auto Result :
match(initListExpr(hasType(equalsNode(RD))).bind("initListExpr"),
  Context))
Index: clang-tools-extra/trunk/test/clang-reorder-fields/PlainCStructFieldsOrder.c
===
--- clang-tools-extra/trunk/test/clang-reorder-fields/PlainCStructFieldsOrder.c
+++ clang-tools-extra/trunk/test/clang-reorder-fields/PlainCStructFieldsOrder.c
@@ -0,0 +1,14 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order z,w,y,x %s -- | FileCheck %s
+
+struct Foo {
+  const int* x; // CHECK:  {{^  double z;}}
+  int y;// CHECK-NEXT: {{^  int w;}}
+  double z; // CHECK-NEXT: {{^  int y;}}
+  int w;// CHECK-NEXT: {{^  const int\* x}}
+};
+
+int main() {
+  const int x = 13;
+  struct Foo foo = { &x, 0, 1.29, 17 }; // CHECK: {{^  struct Foo foo = { 1.29, 17, 0, &x };}} 
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.

[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D35329



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

[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap updated this revision to Diff 107143.
alexshap added a comment.

update the comments


Repository:
  rL LLVM

https://reviews.llvm.org/D35329

Files:
  clang-reorder-fields/ReorderFieldsAction.cpp
  test/clang-reorder-fields/PlainCStructFieldsOrder.c

Index: test/clang-reorder-fields/PlainCStructFieldsOrder.c
===
--- test/clang-reorder-fields/PlainCStructFieldsOrder.c
+++ test/clang-reorder-fields/PlainCStructFieldsOrder.c
@@ -0,0 +1,14 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order z,w,y,x %s -- | FileCheck %s
+
+struct Foo {
+  const int* x; // CHECK:  {{^  double z;}}
+  int y;// CHECK-NEXT: {{^  int w;}}
+  double z; // CHECK-NEXT: {{^  int y;}}
+  int w;// CHECK-NEXT: {{^  const int\* x}}
+};
+
+int main() {
+  const int x = 13;
+  struct Foo foo = { &x, 0, 1.29, 17 }; // CHECK: {{^  struct Foo foo = { 1.29, 17, 0, &x };}} 
+  return 0;
+}
Index: clang-reorder-fields/ReorderFieldsAction.cpp
===
--- clang-reorder-fields/ReorderFieldsAction.cpp
+++ clang-reorder-fields/ReorderFieldsAction.cpp
@@ -32,10 +32,10 @@
 /// \brief Finds the definition of a record by name.
 ///
 /// \returns nullptr if the name is ambiguous or not found.
-static const CXXRecordDecl *findDefinition(StringRef RecordName,
-   ASTContext &Context) {
+static const RecordDecl *findDefinition(StringRef RecordName,
+ASTContext &Context) {
   auto Results = match(
-  recordDecl(hasName(RecordName), isDefinition()).bind("cxxRecordDecl"),
+  recordDecl(hasName(RecordName), isDefinition()).bind("recordDecl"),
   Context);
   if (Results.empty()) {
 llvm::errs() << "Definition of " << RecordName << "  not found\n";
@@ -46,14 +46,14 @@
  << " is ambiguous, several definitions found\n";
 return nullptr;
   }
-  return selectFirst("cxxRecordDecl", Results);
+  return selectFirst("recordDecl", Results);
 }
 
 /// \brief Calculates the new order of fields.
 ///
 /// \returns empty vector if the list of fields doesn't match the definition.
 static SmallVector
-getNewFieldsOrder(const CXXRecordDecl *Definition,
+getNewFieldsOrder(const RecordDecl *Definition,
   ArrayRef DesiredFieldsOrder) {
   assert(Definition && "Definition is null");
 
@@ -97,7 +97,7 @@
 /// different accesses (public/protected/private) is not supported.
 /// \returns true on success.
 static bool reorderFieldsInDefinition(
-const CXXRecordDecl *Definition, ArrayRef NewFieldsOrder,
+const RecordDecl *Definition, ArrayRef NewFieldsOrder,
 const ASTContext &Context,
 std::map &Replacements) {
   assert(Definition && "Definition is null");
@@ -223,25 +223,30 @@
   ReorderingConsumer &operator=(const ReorderingConsumer &) = delete;
 
   void HandleTranslationUnit(ASTContext &Context) override {
-const CXXRecordDecl *RD = findDefinition(RecordName, Context);
+const RecordDecl *RD = findDefinition(RecordName, Context);
 if (!RD)
   return;
 SmallVector NewFieldsOrder =
 getNewFieldsOrder(RD, DesiredFieldsOrder);
 if (NewFieldsOrder.empty())
   return;
 if (!reorderFieldsInDefinition(RD, NewFieldsOrder, Context, Replacements))
   return;
-for (const auto *C : RD->ctors())
-  if (const auto *D = dyn_cast(C->getDefinition()))
-reorderFieldsInConstructor(cast(D),
-   NewFieldsOrder, Context, Replacements);
 
-// We only need to reorder init list expressions for aggregate types.
+// CXXRD will be nullptr if C code (not C++) is being processed.
+const CXXRecordDecl *CXXRD = dyn_cast(RD);
+if (CXXRD)
+  for (const auto *C : CXXRD->ctors())
+if (const auto *D = dyn_cast(C->getDefinition()))
+  reorderFieldsInConstructor(cast(D),
+  NewFieldsOrder, Context, Replacements);
+
+// We only need to reorder init list expressions for 
+// plain C structs or C++ aggregate types.
 // For other types the order of constructor parameters is used,
 // which we don't change at the moment.
 // Now (v0) partial initialization is not supported.
-if (RD->isAggregate())
+if (!CXXRD || CXXRD->isAggregate())
   for (auto Result :
match(initListExpr(hasType(equalsNode(RD))).bind("initListExpr"),
  Context))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-reorder-fields/ReorderFieldsAction.cpp:236
+
+// CXXRD will be nullptr if C code (not C++) is being processed 
+const CXXRecordDecl *CXXRD = dyn_cast(RD);

Nit:  missing a trailing `.`.

I think we could use `Context.getLangOpts()` to determine whether the file 
being processed is C or C++. Use early return If the code is C.


Repository:
  rL LLVM

https://reviews.llvm.org/D35329



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

[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap added a comment.

ping


Repository:
  rL LLVM

https://reviews.llvm.org/D35329



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

[PATCH] D35329: [clang-reorder-fields] Enable reordering for plain C structs

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexshap created this revision.

This diff adds support for reordering fields in structs when the code compiles 
as plain C,
in particular we switch to using RecordDecl instead of CXXRecordDecl where it's 
appropriate.

Test plan: make check-all


Repository:
  rL LLVM

https://reviews.llvm.org/D35329

Files:
  clang-reorder-fields/ReorderFieldsAction.cpp
  test/clang-reorder-fields/PlainCStructFieldsOrder.c


Index: test/clang-reorder-fields/PlainCStructFieldsOrder.c
===
--- test/clang-reorder-fields/PlainCStructFieldsOrder.c
+++ test/clang-reorder-fields/PlainCStructFieldsOrder.c
@@ -0,0 +1,14 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order z,w,y,x %s -- | 
FileCheck %s
+
+struct Foo {
+  const int* x; // CHECK:  {{^  double z;}}
+  int y;// CHECK-NEXT: {{^  int w;}}
+  double z; // CHECK-NEXT: {{^  int y;}}
+  int w;// CHECK-NEXT: {{^  const int\* x}}
+};
+
+int main() {
+  const int x = 13;
+  struct Foo foo = { &x, 0, 1.29, 17 }; // CHECK: {{^  struct Foo foo = { 
1.29, 17, 0, &x };}} 
+  return 0;
+}
Index: clang-reorder-fields/ReorderFieldsAction.cpp
===
--- clang-reorder-fields/ReorderFieldsAction.cpp
+++ clang-reorder-fields/ReorderFieldsAction.cpp
@@ -32,10 +32,10 @@
 /// \brief Finds the definition of a record by name.
 ///
 /// \returns nullptr if the name is ambiguous or not found.
-static const CXXRecordDecl *findDefinition(StringRef RecordName,
-   ASTContext &Context) {
+static const RecordDecl *findDefinition(StringRef RecordName,
+ASTContext &Context) {
   auto Results = match(
-  recordDecl(hasName(RecordName), isDefinition()).bind("cxxRecordDecl"),
+  recordDecl(hasName(RecordName), isDefinition()).bind("recordDecl"),
   Context);
   if (Results.empty()) {
 llvm::errs() << "Definition of " << RecordName << "  not found\n";
@@ -46,14 +46,14 @@
  << " is ambiguous, several definitions found\n";
 return nullptr;
   }
-  return selectFirst("cxxRecordDecl", Results);
+  return selectFirst("recordDecl", Results);
 }
 
 /// \brief Calculates the new order of fields.
 ///
 /// \returns empty vector if the list of fields doesn't match the definition.
 static SmallVector
-getNewFieldsOrder(const CXXRecordDecl *Definition,
+getNewFieldsOrder(const RecordDecl *Definition,
   ArrayRef DesiredFieldsOrder) {
   assert(Definition && "Definition is null");
 
@@ -97,7 +97,7 @@
 /// different accesses (public/protected/private) is not supported.
 /// \returns true on success.
 static bool reorderFieldsInDefinition(
-const CXXRecordDecl *Definition, ArrayRef NewFieldsOrder,
+const RecordDecl *Definition, ArrayRef NewFieldsOrder,
 const ASTContext &Context,
 std::map &Replacements) {
   assert(Definition && "Definition is null");
@@ -223,25 +223,29 @@
   ReorderingConsumer &operator=(const ReorderingConsumer &) = delete;
 
   void HandleTranslationUnit(ASTContext &Context) override {
-const CXXRecordDecl *RD = findDefinition(RecordName, Context);
+const RecordDecl *RD = findDefinition(RecordName, Context);
 if (!RD)
   return;
 SmallVector NewFieldsOrder =
 getNewFieldsOrder(RD, DesiredFieldsOrder);
 if (NewFieldsOrder.empty())
   return;
 if (!reorderFieldsInDefinition(RD, NewFieldsOrder, Context, Replacements))
   return;
-for (const auto *C : RD->ctors())
-  if (const auto *D = dyn_cast(C->getDefinition()))
-reorderFieldsInConstructor(cast(D),
-   NewFieldsOrder, Context, Replacements);
+
+// CXXRD will be nullptr if C code (not C++) is being processed 
+const CXXRecordDecl *CXXRD = dyn_cast(RD);
+if (CXXRD)
+  for (const auto *C : CXXRD->ctors())
+if (const auto *D = dyn_cast(C->getDefinition()))
+  reorderFieldsInConstructor(cast(D),
+  NewFieldsOrder, Context, Replacements);
 
 // We only need to reorder init list expressions for aggregate types.
 // For other types the order of constructor parameters is used,
 // which we don't change at the moment.
 // Now (v0) partial initialization is not supported.
-if (RD->isAggregate())
+if (!CXXRD || CXXRD->isAggregate())
   for (auto Result :
match(initListExpr(hasType(equalsNode(RD))).bind("initListExpr"),
  Context))


Index: test/clang-reorder-fields/PlainCStructFieldsOrder.c
===
--- test/clang-reorder-fields/PlainCStructFieldsOrder.c
+++ test/clang-reorder-fields/PlainCStructFieldsOrder.c
@@ -0,0 +1,14 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order z,w,y,x %s -- | FileCheck %s
+
+struct Foo {
+  const int* x; // CHECK:  {{^  double z;}}
+  int y;