[clang-tools-extra] [clang-reorder-fields] Do not reorder macro expansions. (PR #122877)

Clement Courbet via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 14 01:55:13 PST 2025


https://github.com/legrosbuffle created https://github.com/llvm/llvm-project/pull/122877

When a field definition comes from a macro expansion, reordering completely messes up the file. There is no good way to fix this, since we can't modify the macro itself. Emit a warning instead.

>From 54dfc73006f58c8ac775a398d414131e258523b6 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Tue, 14 Jan 2025 08:52:44 +0000
Subject: [PATCH] [clang-reorder-fields] Do not reorder macro expansions.

When a field definition comes from a macro expansion, reordering
completely messes up the file. There is no good way to fix this, since
we can't modify the macro itself. Emit a warning instead.
---
 .../clang-reorder-fields/ReorderFieldsAction.cpp | 16 ++++++++++++----
 .../clang-reorder-fields/MacroExpansions.cpp     | 12 ++++++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)
 create mode 100644 clang-tools-extra/test/clang-reorder-fields/MacroExpansions.cpp

diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index dc3a3b6211b7e4..9aebfaec51ff66 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -167,10 +167,18 @@ static bool reorderFieldsInDefinition(
     const auto FieldIndex = Field->getFieldIndex();
     if (FieldIndex == NewFieldsOrder[FieldIndex])
       continue;
-    addReplacement(
-        getFullFieldSourceRange(*Field, Context),
-        getFullFieldSourceRange(*Fields[NewFieldsOrder[FieldIndex]], Context),
-        Context, Replacements);
+
+    const auto FromRange = getFullFieldSourceRange(*Field, Context);
+    const auto ToRange =
+        getFullFieldSourceRange(*Fields[NewFieldsOrder[FieldIndex]], Context);
+
+    if (FromRange.getBegin().isMacroID()) {
+      llvm::errs() << "Cannot reorder field `" << Field->getName()
+                   << "` defined within a macro expansion\n";
+      return false;
+    }
+
+    addReplacement(FromRange, ToRange, Context, Replacements);
   }
   return true;
 }
diff --git a/clang-tools-extra/test/clang-reorder-fields/MacroExpansions.cpp b/clang-tools-extra/test/clang-reorder-fields/MacroExpansions.cpp
new file mode 100644
index 00000000000000..58fbd421244b2e
--- /dev/null
+++ b/clang-tools-extra/test/clang-reorder-fields/MacroExpansions.cpp
@@ -0,0 +1,12 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order y,x %s -- | FileCheck %s
+
+#define DEFINE_FIELD(name) \
+    int name
+
+// We should not reorder given that the definition of `x` is in a macro
+// expansion.
+class Foo {
+  DEFINE_FIELD(x); // CHECK:      DEFINE_FIELD(x);
+  int y;           // CHECK-NEXT: int y;
+};
+



More information about the cfe-commits mailing list