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

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


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Clement Courbet (legrosbuffle)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/122877.diff


2 Files Affected:

- (modified) clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp (+12-4) 
- (added) clang-tools-extra/test/clang-reorder-fields/MacroExpansions.cpp (+12) 


``````````diff
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;
+};
+

``````````

</details>


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


More information about the cfe-commits mailing list