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

Alexander Shaposhnikov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 12 14:40:55 PDT 2017


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>("cxxRecordDecl", Results);
+  return selectFirst<RecordDecl>("recordDecl", Results);
 }
 
 /// \brief Calculates the new order of fields.
 ///
 /// \returns empty vector if the list of fields doesn't match the definition.
 static SmallVector<unsigned, 4>
-getNewFieldsOrder(const CXXRecordDecl *Definition,
+getNewFieldsOrder(const RecordDecl *Definition,
                   ArrayRef<std::string> 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<unsigned> NewFieldsOrder,
+    const RecordDecl *Definition, ArrayRef<unsigned> NewFieldsOrder,
     const ASTContext &Context,
     std::map<std::string, tooling::Replacements> &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<unsigned, 4> 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<CXXConstructorDecl>(C->getDefinition()))
-        reorderFieldsInConstructor(cast<const CXXConstructorDecl>(D),
-                                   NewFieldsOrder, Context, Replacements);
+
+    // CXXRD will be nullptr if C code (not C++) is being processed 
+    const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD);
+    if (CXXRD)
+      for (const auto *C : CXXRD->ctors())
+        if (const auto *D = dyn_cast<CXXConstructorDecl>(C->getDefinition()))
+          reorderFieldsInConstructor(cast<const CXXConstructorDecl>(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))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35329.106312.patch
Type: text/x-patch
Size: 4386 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170712/1868ef3e/attachment.bin>


More information about the cfe-commits mailing list