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

[alexander-shaposhnikov]

Summary

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

[alexander-shaposhnikov]

ping

[hokein]

clang-reorder-fields/ReorderFieldsAction.cpp

236 ↗

(On Diff #106312)

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.

[alexander-shaposhnikov]

update the comments

[hokein]

LGTM.

clang-tools-extra/trunk/clang-reorder-fields/ReorderFieldsAction.cpp

 
 namespace clang {
 namespace reorder_fields {
 using namespace clang::ast_matchers;
 
 /// \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,
   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";
     return nullptr;
   }
   if (Results.size() > 1) {
     llvm::errs() << "The name " << RecordName
                  << " 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");
 
   llvm::StringMap<unsigned> NameToIndex;
   for (const auto *Field : Definition->fields())
     NameToIndex[Field->getName()] = Field->getFieldIndex();
 
   if (DesiredFieldsOrder.size() != NameToIndex.size()) {

 }
 
 /// \brief Reorders fields in the definition of a struct/class.
 ///
 /// At the moment reodering of fields with
 /// 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");
 
   SmallVector<const FieldDecl *, 10> Fields;
   for (const auto *Field : Definition->fields())
     Fields.push_back(Field);
 
ReorderingConsumer(StringRef RecordName,
                      std::map<std::string, tooling::Replacements> &Replacements)
       : RecordName(RecordName), DesiredFieldsOrder(DesiredFieldsOrder),
         Replacements(Replacements) {}
 
   ReorderingConsumer(const ReorderingConsumer &) = delete;
   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())
+
+    // 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.
+    // 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))
         if (!reorderFieldsInInitListExpr(
                 Result.getNodeAs<InitListExpr>("initListExpr"), NewFieldsOrder,
                 Context, Replacements)) {
           Replacements.clear();
           return;

clang-tools-extra/trunk/test/clang-reorder-fields/PlainCStructFieldsOrder.c

+// 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;
+}