Skip to content

Conversation

@vvuksanovic
Copy link
Contributor

Initializer lists with designators, missing elements or omitted braces can now be rewritten. Any missing designators are added and they get sorted according to the new order.

struct Foo {
  int a;
  int b;
  int c;
};
struct Foo foo = { .a = 1, 2, 3 }

when reordering elements to "b,a,c" becomes:

struct Foo {
  int b;
  int a;
  int c;
};
struct Foo foo = { .b = 2, .a = 1, .c = 3 }

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented May 30, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: Vladimir Vuksanovic (vvuksanovic)

Changes

Initializer lists with designators, missing elements or omitted braces can now be rewritten. Any missing designators are added and they get sorted according to the new order.

struct Foo {
  int a;
  int b;
  int c;
};
struct Foo foo = { .a = 1, 2, 3 }

when reordering elements to "b,a,c" becomes:

struct Foo {
  int b;
  int a;
  int c;
};
struct Foo foo = { .b = 2, .a = 1, .c = 3 }

Patch is 34.11 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/142150.diff

5 Files Affected:

  • (modified) clang-tools-extra/clang-reorder-fields/CMakeLists.txt (+1)
  • (modified) clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp (+253-48)
  • (added) clang-tools-extra/clang-reorder-fields/utils/Designator.cpp (+256)
  • (added) clang-tools-extra/clang-reorder-fields/utils/Designator.h (+118)
  • (added) clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.c (+31)
diff --git a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
index 2fdeb65d89767..dfb28234fd548 100644
--- a/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
+++ b/clang-tools-extra/clang-reorder-fields/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
 )
 
 add_clang_library(clangReorderFields STATIC
+  utils/Designator.cpp
   ReorderFieldsAction.cpp
 
   DEPENDS
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index ea0207619fb2b..f5961a7dab0c9 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ReorderFieldsAction.h"
+#include "utils/Designator.h"
 #include "clang/AST/AST.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
@@ -23,6 +24,7 @@
 #include "clang/Tooling/Refactoring.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/Support/ErrorHandling.h"
 #include <string>
 
 namespace clang {
@@ -81,7 +83,44 @@ getNewFieldsOrder(const RecordDecl *Definition,
   return NewFieldsOrder;
 }
 
+struct ReorderedStruct {
+public:
+  ReorderedStruct(const RecordDecl *Decl, ArrayRef<unsigned> NewFieldsOrder)
+      : Definition(Decl), NewFieldsOrder(NewFieldsOrder),
+        NewFieldsPositions(NewFieldsOrder.size()) {
+    for (unsigned I = 0; I < NewFieldsPositions.size(); ++I)
+      NewFieldsPositions[NewFieldsOrder[I]] = I;
+  }
+
+  const RecordDecl *Definition;
+  ArrayRef<unsigned> NewFieldsOrder;
+  SmallVector<unsigned, 4> NewFieldsPositions;
+};
+
 // FIXME: error-handling
+/// Replaces a range of source code by the specified text.
+static void
+addReplacement(SourceRange Old, StringRef New, const ASTContext &Context,
+               std::map<std::string, tooling::Replacements> &Replacements) {
+  tooling::Replacement R(Context.getSourceManager(),
+                         CharSourceRange::getTokenRange(Old), New,
+                         Context.getLangOpts());
+  consumeError(Replacements[std::string(R.getFilePath())].add(R));
+}
+
+/// Replaces one range of source code by another and adds a prefix.
+static void
+addReplacement(SourceRange Old, SourceRange New, StringRef Prefix,
+               const ASTContext &Context,
+               std::map<std::string, tooling::Replacements> &Replacements) {
+  std::string NewText =
+      (Prefix + Lexer::getSourceText(CharSourceRange::getTokenRange(New),
+                                     Context.getSourceManager(),
+                                     Context.getLangOpts()))
+          .str();
+  addReplacement(Old, NewText, Context, Replacements);
+}
+
 /// Replaces one range of source code by another.
 static void
 addReplacement(SourceRange Old, SourceRange New, const ASTContext &Context,
@@ -89,10 +128,7 @@ addReplacement(SourceRange Old, SourceRange New, const ASTContext &Context,
   StringRef NewText =
       Lexer::getSourceText(CharSourceRange::getTokenRange(New),
                            Context.getSourceManager(), Context.getLangOpts());
-  tooling::Replacement R(Context.getSourceManager(),
-                         CharSourceRange::getTokenRange(Old), NewText,
-                         Context.getLangOpts());
-  consumeError(Replacements[std::string(R.getFilePath())].add(R));
+  addReplacement(Old, NewText.str(), Context, Replacements);
 }
 
 /// Find all member fields used in the given init-list initializer expr
@@ -194,33 +230,33 @@ static SourceRange getFullFieldSourceRange(const FieldDecl &Field,
 /// different accesses (public/protected/private) is not supported.
 /// \returns true on success.
 static bool reorderFieldsInDefinition(
-    const RecordDecl *Definition, ArrayRef<unsigned> NewFieldsOrder,
-    const ASTContext &Context,
+    const ReorderedStruct &RS, const ASTContext &Context,
     std::map<std::string, tooling::Replacements> &Replacements) {
-  assert(Definition && "Definition is null");
+  assert(RS.Definition && "Definition is null");
 
   SmallVector<const FieldDecl *, 10> Fields;
-  for (const auto *Field : Definition->fields())
+  for (const auto *Field : RS.Definition->fields())
     Fields.push_back(Field);
 
   // Check that the permutation of the fields doesn't change the accesses
-  for (const auto *Field : Definition->fields()) {
+  for (const auto *Field : RS.Definition->fields()) {
     const auto FieldIndex = Field->getFieldIndex();
-    if (Field->getAccess() != Fields[NewFieldsOrder[FieldIndex]]->getAccess()) {
+    if (Field->getAccess() !=
+        Fields[RS.NewFieldsOrder[FieldIndex]]->getAccess()) {
       llvm::errs() << "Currently reordering of fields with different accesses "
                       "is not supported\n";
       return false;
     }
   }
 
-  for (const auto *Field : Definition->fields()) {
+  for (const auto *Field : RS.Definition->fields()) {
     const auto FieldIndex = Field->getFieldIndex();
-    if (FieldIndex == NewFieldsOrder[FieldIndex])
+    if (FieldIndex == RS.NewFieldsOrder[FieldIndex])
       continue;
-    addReplacement(
-        getFullFieldSourceRange(*Field, Context),
-        getFullFieldSourceRange(*Fields[NewFieldsOrder[FieldIndex]], Context),
-        Context, Replacements);
+    addReplacement(getFullFieldSourceRange(*Field, Context),
+                   getFullFieldSourceRange(
+                       *Fields[RS.NewFieldsOrder[FieldIndex]], Context),
+                   Context, Replacements);
   }
   return true;
 }
@@ -231,7 +267,7 @@ static bool reorderFieldsInDefinition(
 /// fields. Thus, we need to ensure that we reorder just the initializers that
 /// are present.
 static void reorderFieldsInConstructor(
-    const CXXConstructorDecl *CtorDecl, ArrayRef<unsigned> NewFieldsOrder,
+    const CXXConstructorDecl *CtorDecl, const ReorderedStruct &RS,
     ASTContext &Context,
     std::map<std::string, tooling::Replacements> &Replacements) {
   assert(CtorDecl && "Constructor declaration is null");
@@ -243,10 +279,6 @@ static void reorderFieldsInConstructor(
   // Thus this assert needs to be after the previous checks.
   assert(CtorDecl->isThisDeclarationADefinition() && "Not a definition");
 
-  SmallVector<unsigned, 10> NewFieldsPositions(NewFieldsOrder.size());
-  for (unsigned i = 0, e = NewFieldsOrder.size(); i < e; ++i)
-    NewFieldsPositions[NewFieldsOrder[i]] = i;
-
   SmallVector<const CXXCtorInitializer *, 10> OldWrittenInitializersOrder;
   SmallVector<const CXXCtorInitializer *, 10> NewWrittenInitializersOrder;
   for (const auto *Initializer : CtorDecl->inits()) {
@@ -257,8 +289,8 @@ static void reorderFieldsInConstructor(
     const FieldDecl *ThisM = Initializer->getMember();
     const auto UsedMembers = findMembersUsedInInitExpr(Initializer, Context);
     for (const FieldDecl *UM : UsedMembers) {
-      if (NewFieldsPositions[UM->getFieldIndex()] >
-          NewFieldsPositions[ThisM->getFieldIndex()]) {
+      if (RS.NewFieldsPositions[UM->getFieldIndex()] >
+          RS.NewFieldsPositions[ThisM->getFieldIndex()]) {
         DiagnosticsEngine &DiagEngine = Context.getDiagnostics();
         auto Description = ("reordering field " + UM->getName() + " after " +
                             ThisM->getName() + " makes " + UM->getName() +
@@ -276,8 +308,8 @@ static void reorderFieldsInConstructor(
   auto ByFieldNewPosition = [&](const CXXCtorInitializer *LHS,
                                 const CXXCtorInitializer *RHS) {
     assert(LHS && RHS);
-    return NewFieldsPositions[LHS->getMember()->getFieldIndex()] <
-           NewFieldsPositions[RHS->getMember()->getFieldIndex()];
+    return RS.NewFieldsPositions[LHS->getMember()->getFieldIndex()] <
+           RS.NewFieldsPositions[RHS->getMember()->getFieldIndex()];
   };
   llvm::sort(NewWrittenInitializersOrder, ByFieldNewPosition);
   assert(OldWrittenInitializersOrder.size() ==
@@ -289,35 +321,205 @@ static void reorderFieldsInConstructor(
                      Replacements);
 }
 
+/// Replacement for broken InitListExpr::isExplicit function.
+/// TODO: Remove when InitListExpr::isExplicit is fixed.
+static bool isImplicitILE(const InitListExpr *ILE, const ASTContext &Context) {
+  // The ILE is implicit if either:
+  // - The left brace loc of the ILE matches the start of first init expression
+  //   (for non designated decls)
+  // - The right brace loc of the ILE matches the end of first init expression
+  //   (for designated decls)
+  // The first init expression should be taken from the syntactic form, but
+  // since the ILE could be implicit, there might not be a syntactic form.
+  // For that reason we have to check against all init expressions.
+  for (const Expr *Init : ILE->inits()) {
+    if (ILE->getLBraceLoc() == Init->getBeginLoc() ||
+        ILE->getRBraceLoc() == Init->getEndLoc())
+      return true;
+  }
+  return false;
+}
+
+/// Compares compatible designators according to the new struct order.
+/// Returns a negative value if Lhs < Rhs, positive value if Lhs > Rhs and 0 if
+/// they are equal.
+static int cmpDesignators(const DesignatorIter &Lhs, const DesignatorIter &Rhs,
+                          const ReorderedStruct &Struct) {
+  assert(Lhs.getTag() == Rhs.getTag() && "Incompatible designators");
+  switch (Lhs.getTag()) {
+  case DesignatorIter::STRUCT: {
+    assert(Lhs.getStructDecl() == Rhs.getStructDecl() &&
+           "Incompatible structs");
+    // Use the new layout for reordered struct.
+    if (Struct.Definition == Lhs.getStructDecl()) {
+      return Struct.NewFieldsPositions[Lhs.getStructIter()->getFieldIndex()] -
+             Struct.NewFieldsPositions[Rhs.getStructIter()->getFieldIndex()];
+    }
+    return Lhs.getStructIter()->getFieldIndex() -
+           Rhs.getStructIter()->getFieldIndex();
+  }
+  case DesignatorIter::ARRAY:
+    return Lhs.getArrayIndex() - Rhs.getArrayIndex();
+  case DesignatorIter::ARRAY_RANGE:
+    return Lhs.getArrayRangeEnd() - Rhs.getArrayRangeEnd();
+  }
+  llvm_unreachable("Invalid designator tag");
+}
+
+/// Compares compatible designator lists according to the new struct order.
+/// Returns a negative value if Lhs < Rhs, positive value if Lhs > Rhs and 0 if
+/// they are equal.
+static int cmpDesignatorLists(const Designators &Lhs, const Designators &Rhs,
+                              const ReorderedStruct &Reorders) {
+  for (unsigned Idx = 0, Size = std::min(Lhs.size(), Rhs.size()); Idx < Size;
+       ++Idx) {
+    int DesignatorComp = cmpDesignators(Lhs[Idx], Rhs[Idx], Reorders);
+    // If the current designators are not equal, return the result
+    if (DesignatorComp != 0)
+      return DesignatorComp;
+    // Otherwise, continue to the next pair.
+  }
+  //
+  return Lhs.size() - Rhs.size();
+}
+
+/// Finds the semantic form of the first explicit ancestor of the given
+/// initializer list including itself.
+static const InitListExpr *getExplicitILE(const InitListExpr *ILE,
+                                          ASTContext &Context) {
+  if (!isImplicitILE(ILE, Context))
+    return ILE;
+  const InitListExpr *TopLevelILE = ILE;
+  DynTypedNodeList Parents = Context.getParents(*TopLevelILE);
+  while (!Parents.empty() && Parents.begin()->get<InitListExpr>()) {
+    TopLevelILE = Parents.begin()->get<InitListExpr>();
+    Parents = Context.getParents(*TopLevelILE);
+    if (!isImplicitILE(TopLevelILE, Context))
+      break;
+  }
+  if (!TopLevelILE->isSemanticForm()) {
+    return TopLevelILE->getSemanticForm();
+  }
+  return TopLevelILE;
+}
+
 /// Reorders initializers in the brace initialization of an aggregate.
 ///
 /// At the moment partial initialization is not supported.
 /// \returns true on success
 static bool reorderFieldsInInitListExpr(
-    const InitListExpr *InitListEx, ArrayRef<unsigned> NewFieldsOrder,
-    const ASTContext &Context,
+    const InitListExpr *InitListEx, const ReorderedStruct &RS,
+    ASTContext &Context,
     std::map<std::string, tooling::Replacements> &Replacements) {
   assert(InitListEx && "Init list expression is null");
-  // We care only about InitListExprs which originate from source code.
-  // Implicit InitListExprs are created by the semantic analyzer.
-  if (!InitListEx->isExplicit())
+  // Only process semantic forms of initializer lists.
+  if (!InitListEx->isSemanticForm()) {
     return true;
-  // The method InitListExpr::getSyntacticForm may return nullptr indicating
-  // that the current initializer list also serves as its syntactic form.
-  if (const auto *SyntacticForm = InitListEx->getSyntacticForm())
-    InitListEx = SyntacticForm;
+  }
+
   // If there are no initializers we do not need to change anything.
   if (!InitListEx->getNumInits())
     return true;
-  if (InitListEx->getNumInits() != NewFieldsOrder.size()) {
-    llvm::errs() << "Currently only full initialization is supported\n";
-    return false;
+
+  // We care only about InitListExprs which originate from source code.
+  // Implicit InitListExprs are created by the semantic analyzer.
+  // We find the first parent InitListExpr that exists in source code and
+  // process it. This is necessary because of designated initializer lists and
+  // possible omitted braces.
+  InitListEx = getExplicitILE(InitListEx, Context);
+
+  // Find if there are any designated initializations or implicit values. If all
+  // initializers are present and none have designators then just reorder them
+  // normally. Otherwise, designators are added to all initializers and they are
+  // sorted in the new order.
+  bool ShouldAddDesignators = false;
+  // The method InitListExpr::getSyntacticForm may return nullptr indicating
+  // that the current initializer list also serves as its syntactic form.
+  const InitListExpr *SyntacticInitListEx = InitListEx;
+  if (const InitListExpr *SynILE = InitListEx->getSyntacticForm()) {
+    // Do not rewrite zero initializers. This check is only valid for syntactic
+    // forms.
+    if (SynILE->isIdiomaticZeroInitializer(Context.getLangOpts()))
+      return true;
+
+    ShouldAddDesignators = InitListEx->getNumInits() != SynILE->getNumInits() ||
+                           llvm::any_of(SynILE->inits(), [](const Expr *Init) {
+                             return isa<DesignatedInitExpr>(Init);
+                           });
+
+    SyntacticInitListEx = SynILE;
+  } else {
+    // If there is no syntactic form, there can be no designators. Instead,
+    // there might be implicit values.
+    ShouldAddDesignators =
+        (RS.NewFieldsOrder.size() != InitListEx->getNumInits()) ||
+        llvm::any_of(InitListEx->inits(), [&Context](const Expr *Init) {
+          return isa<ImplicitValueInitExpr>(Init) ||
+                 (isa<InitListExpr>(Init) &&
+                  isImplicitILE(dyn_cast<InitListExpr>(Init), Context));
+        });
+  }
+
+  if (ShouldAddDesignators) {
+    // Designators are only supported from C++20.
+    if (Context.getLangOpts().CPlusPlus && !Context.getLangOpts().CPlusPlus20) {
+      llvm::errs()
+          << "Currently only full initialization is supported for C++\n";
+      return false;
+    }
+
+    // Handle case when some fields are designated. Some fields can be
+    // missing. Insert any missing designators and reorder the expressions
+    // according to the new order.
+    Designators CurrentDesignators{};
+    // Remember each initializer expression along with its designators. They are
+    // sorted later to determine the correct order.
+    std::vector<std::pair<Designators, const Expr *>> Rewrites;
+    for (const Expr *Init : SyntacticInitListEx->inits()) {
+      if (const auto *DIE = dyn_cast_or_null<DesignatedInitExpr>(Init)) {
+        CurrentDesignators = {DIE, SyntacticInitListEx, Context};
+
+        // Use the child of the DesignatedInitExpr. This way designators are
+        // always replaced.
+        Rewrites.push_back({CurrentDesignators, DIE->getInit()});
+      } else {
+        // Find the next field.
+        if (!CurrentDesignators.increment(SyntacticInitListEx, Init, Context)) {
+          llvm::errs() << "Unsupported initializer list\n";
+          return false;
+        }
+
+        // Do not rewrite implicit values. They just had to be processed to
+        // find the correct designator.
+        if (!isa<ImplicitValueInitExpr>(Init))
+          Rewrites.push_back({CurrentDesignators, Init});
+      }
+    }
+
+    // Sort the designators according to the new order.
+    llvm::sort(Rewrites, [&RS](const auto &Lhs, const auto &Rhs) {
+      return cmpDesignatorLists(Lhs.first, Rhs.first, RS) < 0;
+    });
+
+    for (unsigned i = 0, e = Rewrites.size(); i < e; ++i) {
+      addReplacement(SyntacticInitListEx->getInit(i)->getSourceRange(),
+                     Rewrites[i].second->getSourceRange(),
+                     Rewrites[i].first.toString(), Context, Replacements);
+    }
+  } else {
+    // Handle excess initializers by leaving them unchanged.
+    assert(SyntacticInitListEx->getNumInits() >= InitListEx->getNumInits());
+
+    // All field initializers are present and none have designators. They can be
+    // reordered normally.
+    for (unsigned i = 0, e = RS.NewFieldsOrder.size(); i < e; ++i) {
+      if (i != RS.NewFieldsOrder[i])
+        addReplacement(SyntacticInitListEx->getInit(i)->getSourceRange(),
+                       SyntacticInitListEx->getInit(RS.NewFieldsOrder[i])
+                           ->getSourceRange(),
+                       Context, Replacements);
+    }
   }
-  for (unsigned i = 0, e = InitListEx->getNumInits(); i < e; ++i)
-    if (i != NewFieldsOrder[i])
-      addReplacement(InitListEx->getInit(i)->getSourceRange(),
-                     InitListEx->getInit(NewFieldsOrder[i])->getSourceRange(),
-                     Context, Replacements);
   return true;
 }
 
@@ -345,7 +547,9 @@ class ReorderingConsumer : public ASTConsumer {
         getNewFieldsOrder(RD, DesiredFieldsOrder);
     if (NewFieldsOrder.empty())
       return;
-    if (!reorderFieldsInDefinition(RD, NewFieldsOrder, Context, Replacements))
+    ReorderedStruct RS{RD, NewFieldsOrder};
+
+    if (!reorderFieldsInDefinition(RS, Context, Replacements))
       return;
 
     // CXXRD will be nullptr if C code (not C++) is being processed.
@@ -353,24 +557,25 @@ class ReorderingConsumer : public ASTConsumer {
     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);
+          reorderFieldsInConstructor(cast<const CXXConstructorDecl>(D), RS,
+                                     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 (!CXXRD || CXXRD->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)) {
+                Result.getNodeAs<InitListExpr>("initListExpr"), RS, Context,
+                Replacements)) {
           Replacements.clear();
           return;
         }
+    }
   }
 };
 } // end anonymous namespace
diff --git a/clang-tools-extra/clang-reorder-fields/utils/Designator.cpp b/clang-tools-extra/clang-reorder-fields/utils/Designator.cpp
new file mode 100644
index 0000000000000..9ad60a3fc5db6
--- /dev/null
+++ b/clang-tools-extra/clang-reorder-fields/utils/Designator.cpp
@@ -0,0 +1,256 @@
+//===-- tools/extra/clang-reorder-fields/utils/Designator.cpp ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------...
[truncated]

@vvuksanovic
Copy link
Contributor Author

@legrosbuffle

@vvuksanovic vvuksanovic force-pushed the reorder-fields-designators branch from 67febc3 to df2e394 Compare June 16, 2025 08:19
@vvuksanovic
Copy link
Contributor Author

@alexander-shaposhnikov

Copy link
Contributor

@legrosbuffle legrosbuffle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice ! Conceptually this looks good, only stylistic comments.

@vvuksanovic
Copy link
Contributor Author

ping @legrosbuffle

@alexander-shaposhnikov
Copy link
Collaborator

apologize for the delay, I'll get back to this PR within a couple of days

Move the new fields order information to a new struct.
Initializer lists with designators, missing elements or omitted braces can now be rewritten. Any missing designators are added and they get sorted according to the new order.

```
struct Foo {
int a;
int b;
int c;
};
struct Foo foo = { .a = 1, 2, 3 }
```

when reordering elements to "b,a,c" becomes:

```
struct Foo {
int b;
int a;
int c;
};
struct Foo foo = { .b = 2, .a = 1, .c = 3 }
```
@vvuksanovic vvuksanovic force-pushed the reorder-fields-designators branch from 9766119 to 144f878 Compare July 24, 2025 14:54
@vvuksanovic
Copy link
Contributor Author

Ping

@alexander-shaposhnikov
Copy link
Collaborator

alexander-shaposhnikov commented Aug 21, 2025

I've added a couple of comments, but overall this looks reasonable to me, thank you for your patience

@vvuksanovic
Copy link
Contributor Author

Thank you for the suggestions, the code looks cleaner now. I don't see any new comments?

Copy link
Collaborator

@alexander-shaposhnikov alexander-shaposhnikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (modulo comments)

@alexander-shaposhnikov
Copy link
Collaborator

lgtm

@vvuksanovic
Copy link
Contributor Author

Thank you for your patience. Can you please merge?

@alexander-shaposhnikov alexander-shaposhnikov merged commit 4d9578b into llvm:main Sep 1, 2025
9 checks passed
@github-actions
Copy link

github-actions bot commented Sep 1, 2025

@vvuksanovic Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

@nikic
Copy link
Contributor

nikic commented Sep 1, 2025

Building CXX object tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o

FAILED: tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o
sccache /opt/llvm/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/tools/extra/clang-reorder-fields -I/home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/clang-reorder-fields -I/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -std=c++17  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o -MF tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o.d -o tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o -c /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/clang-reorder-fields/Designator.cpp
In file included from /home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/clang-reorder-fields/Designator.cpp:15:
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang-tools-extra/clang-reorder-fields/Designator.h:157:23: error: private field 'ILE' is not used [-Werror,-Wunused-private-field]
157 |   const InitListExpr *ILE;
|                       ^
1 error generated.

@nikic
Copy link
Contributor

nikic commented Sep 1, 2025

Fixed by c128b8c.

@vvuksanovic
Copy link
Contributor Author

Thanks!

@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 2, 2025

LLVM Buildbot has detected a new failure on builder clang-ppc64le-rhel running on ppc64le-clang-rhel-test while building clang-tools-extra at step 5 "build-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/9503

Here is the relevant piece of the build log for the reference
Step 5 (build-unified-tree) failure: build (failure)
...
51.038 [1031/33/5575] Building CXX object tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/DumpAST.cpp.o
51.071 [1031/32/5576] Building CXX object tools/clang/tools/extra/clangd/tool/CMakeFiles/obj.clangdMain.dir/Check.cpp.o
51.084 [1031/31/5577] Building CXX object tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/ExpandMacro.cpp.o
51.085 [1031/30/5578] Building CXX object tools/clang/tools/extra/clang-query/CMakeFiles/obj.clangQuery.dir/Query.cpp.o
51.093 [1031/29/5579] Linking CXX shared library lib/libclangIncludeCleaner.so.22.0git
51.101 [1030/29/5580] Creating library symlink lib/libclangIncludeCleaner.so
51.120 [1030/28/5581] Building CXX object tools/clang/tools/extra/clangd/tool/CMakeFiles/obj.clangdMain.dir/ClangdMain.cpp.o
51.131 [1030/27/5582] Building CXX object tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/DefineInline.cpp.o
51.292 [1030/26/5583] Building CXX object tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/RemoveUsingNamespace.cpp.o
53.238 [1030/25/5584] Building CXX object tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o
FAILED: tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o 
ccache /home/buildbots/llvm-external-buildbots/clang.19.1.7/bin/clang++ --gcc-toolchain=/gcc-toolchain/usr -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/tools/clang/tools/extra/clang-reorder-fields -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-reorder-fields -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/tools/clang/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -std=c++17  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o -MF tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o.d -o tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/Designator.cpp.o -c /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-reorder-fields/Designator.cpp
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-reorder-fields/Designator.cpp:15:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/clang-tools-extra/clang-reorder-fields/Designator.h:157:23: error: private field 'ILE' is not used [-Werror,-Wunused-private-field]
  157 |   const InitListExpr *ILE;
      |                       ^
1 error generated.
53.523 [1030/24/5585] Building CXX object tools/clang/lib/Driver/CMakeFiles/obj.clangDriver.dir/ToolChains/HIPUtility.cpp.o
53.847 [1030/23/5586] Building RISCVGenInstrInfo.inc...
55.328 [1030/22/5587] Building AArch64GenSubtargetInfo.inc...
57.717 [1030/21/5588] Building AArch64GenInstrInfo.inc...
58.620 [1030/20/5589] Building RISCVGenDAGISel.inc...
61.264 [1030/19/5590] Building CXX object lib/CodeGen/AsmPrinter/CMakeFiles/LLVMAsmPrinter.dir/AsmPrinter.cpp.o
61.634 [1030/18/5591] Building CXX object lib/LTO/CMakeFiles/LLVMLTO.dir/LTO.cpp.o
70.906 [1030/17/5592] Building CXX object tools/clang/tools/extra/clang-reorder-fields/CMakeFiles/obj.clangReorderFields.dir/ReorderFieldsAction.cpp.o
72.550 [1030/16/5593] Building AMDGPUGenMCPseudoLowering.inc...
73.060 [1030/15/5594] Building AMDGPUGenRegBankGICombiner.inc...
73.435 [1030/14/5595] Building AMDGPUGenSubtargetInfo.inc...
76.669 [1030/13/5596] Building AMDGPUGenDisassemblerTables.inc...
77.012 [1030/12/5597] Building AMDGPUGenPreLegalizeGICombiner.inc...
77.407 [1030/11/5598] Building AMDGPUGenPostLegalizeGICombiner.inc...
79.159 [1030/10/5599] Building AMDGPUGenSearchableTables.inc...
79.509 [1030/9/5600] Building AMDGPUGenAsmWriter.inc...
80.157 [1030/8/5601] Building AMDGPUGenMCCodeEmitter.inc...
80.739 [1030/7/5602] Building AMDGPUGenCallingConv.inc...
83.658 [1030/6/5603] Building AMDGPUGenAsmMatcher.inc...
86.245 [1030/5/5604] Building AMDGPUGenGlobalISel.inc...
86.387 [1030/4/5605] Building AMDGPUGenDAGISel.inc...
87.022 [1030/3/5606] Building AMDGPUGenInstrInfo.inc...
94.086 [1030/2/5607] Building AMDGPUGenRegisterBank.inc...
94.535 [1030/1/5608] Building AMDGPUGenRegisterInfo.inc...
ninja: build stopped: subcommand failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants