[clang-reorder-fields] Handle macros fields declarations.

[legrosbuffle]

Right now fields with macro declarations break the tool:

struct Foo {
  Mutex mu;
  int x GUARDED_BY(mu);
  int y;
};

reordered by mu,y,x yields:

struct Foo {
  Mutex mu;
  int y GUARDED_BY(mu);
  int x;
};

[llvmbot]

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

Author: Clement Courbet (legrosbuffle)

Changes

Right now fields with macro declarations break the tool:

struct Foo {
  Mutex mu;
  int x GUARDED_BY(mu);
  int y;
};

reordered by mu,y,x yields:

struct Foo {
  Mutex mu;
  int y GUARDED_BY(mu);
  int x;
};

---

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

2 Files Affected:

• (modified) clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp (+26-3)
• (added) clang-tools-extra/test/clang-reorder-fields/FieldAnnotationsInMacros.cpp (+9)

diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index 346437e20322e6..dc3a3b6211b7e4 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -116,6 +116,28 @@ findMembersUsedInInitExpr(const CXXCtorInitializer *Initializer,
   return Results;
 }
 
+/// Returns the full source range for the field declaration up to (not
+/// including) the trailing semicolumn, including potential macro invocations,
+/// e.g. `int a GUARDED_BY(mu);`.
+static SourceRange getFullFieldSourceRange(const FieldDecl &Field,
+                                           const ASTContext &Context) {
+  SourceRange Range = Field.getSourceRange();
+  SourceLocation End = Range.getEnd();
+  const SourceManager &SM = Context.getSourceManager();
+  const LangOptions &LangOpts = Context.getLangOpts();
+  while (true) {
+    std::optional<Token> CurrentToken = Lexer::findNextToken(End, SM, LangOpts);
+
+    if (!CurrentToken || CurrentToken->is(tok::semi))
+      break;
+
+    if (CurrentToken->is(tok::eof))
+      return Range; // Something is wrong, return the original range.
+    End = CurrentToken->getLastLoc();
+  }
+  return SourceRange(Range.getBegin(), End);
+}
+
 /// Reorders fields in the definition of a struct/class.
 ///
 /// At the moment reordering of fields with
@@ -145,9 +167,10 @@ static bool reorderFieldsInDefinition(
     const auto FieldIndex = Field->getFieldIndex();
     if (FieldIndex == NewFieldsOrder[FieldIndex])
       continue;
-    addReplacement(Field->getSourceRange(),
-                   Fields[NewFieldsOrder[FieldIndex]]->getSourceRange(),
-                   Context, Replacements);
+    addReplacement(
+        getFullFieldSourceRange(*Field, Context),
+        getFullFieldSourceRange(*Fields[NewFieldsOrder[FieldIndex]], Context),
+        Context, Replacements);
   }
   return true;
 }
diff --git a/clang-tools-extra/test/clang-reorder-fields/FieldAnnotationsInMacros.cpp b/clang-tools-extra/test/clang-reorder-fields/FieldAnnotationsInMacros.cpp
new file mode 100644
index 00000000000000..aedec9556aa55a
--- /dev/null
+++ b/clang-tools-extra/test/clang-reorder-fields/FieldAnnotationsInMacros.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order y,x %s -- | FileCheck %s
+
+#define GUARDED_BY(x) __attribute__((guarded_by(x)))
+
+class Foo {
+  int x GUARDED_BY(x); // CHECK: {{^  int y;}}
+  int y;               // CHECK-NEXT: {{^  int x GUARDED_BY\(x\);}}
+};
+

[alexander-shaposhnikov]

LG