https://github.com/vvuksanovic updated
https://github.com/llvm/llvm-project/pull/142149
>From b8481a36b33e71248170c8cc195b45fa9de4f777 Mon Sep 17 00:00:00 2001
From: Vladimir Vuksanovic <vladimir.vuksano...@htecgroup.com>
Date: Fri, 30 May 2025 05:42:55 -0700
Subject: [PATCH 1/3] [clang-reorder-fields] Prevent rewriting unsupported
cases
Add checks to prevent rewriting when doing so might result in incorrect
code. The following cases are checked:
- There are multiple field declarations in one statement like `int a, b`
- Multiple fields are created from a single macro expansion
- Preprocessor directives are present in the struct
---
.../ReorderFieldsAction.cpp | 60 +++++++++++++++++++
.../MacroExpandsToMultipleFields.cpp | 13 ++++
.../MultipleFieldDeclsInStatement.cpp | 11 ++++
.../PreprocessorDirectiveAroundDefinition.cpp | 15 +++++
.../PreprocessorDirectiveAroundFields.cpp | 15 +++++
.../PreprocessorDirectiveInDefinition.cpp | 16 +++++
6 files changed, 130 insertions(+)
create mode 100644
clang-tools-extra/test/clang-reorder-fields/MacroExpandsToMultipleFields.cpp
create mode 100644
clang-tools-extra/test/clang-reorder-fields/MultipleFieldDeclsInStatement.cpp
create mode 100644
clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundDefinition.cpp
create mode 100644
clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
create mode 100644
clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveInDefinition.cpp
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index ea0207619fb2b..25ee1ec815a9f 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -19,6 +19,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/Lexer.h"
#include "clang/Tooling/Refactoring.h"
#include "llvm/ADT/STLExtras.h"
@@ -50,6 +51,63 @@ static const RecordDecl *findDefinition(StringRef RecordName,
return selectFirst<RecordDecl>("recordDecl", Results);
}
+static bool isSafeToRewrite(const RecordDecl *Decl, const ASTContext &Context)
{
+ // All following checks expect at least one field declaration.
+ if (Decl->field_empty())
+ return true;
+
+ // Don't attempt to rewrite if there is a declaration like 'int a, b;'.
+ SourceLocation LastTypeLoc;
+ for (const auto &Field : Decl->fields()) {
+ SourceLocation TypeLoc =
+ Field->getTypeSourceInfo()->getTypeLoc().getBeginLoc();
+ if (LastTypeLoc.isValid() && TypeLoc == LastTypeLoc)
+ return false;
+ LastTypeLoc = TypeLoc;
+ }
+
+ // Don't attempt to rewrite if a single macro expansion creates multiple
+ // fields.
+ SourceLocation LastMacroLoc;
+ for (const auto &Field : Decl->fields()) {
+ if (!Field->getLocation().isMacroID())
+ continue;
+ SourceLocation MacroLoc =
+ Context.getSourceManager().getExpansionLoc(Field->getLocation());
+ if (LastMacroLoc.isValid() && MacroLoc == LastMacroLoc)
+ return false;
+ LastMacroLoc = MacroLoc;
+ }
+
+ // Prevent rewriting if there are preprocessor directives present between the
+ // start of the first field and the end of last field.
+ const SourceManager &SM = Context.getSourceManager();
+ std::pair<FileID, unsigned> FileAndOffset =
+ SM.getDecomposedLoc(Decl->field_begin()->getBeginLoc());
+ auto LastField = Decl->field_begin();
+ while (std::next(LastField) != Decl->field_end())
+ ++LastField;
+ unsigned EndOffset = SM.getFileOffset(LastField->getEndLoc());
+ StringRef SrcBuffer = SM.getBufferData(FileAndOffset.first);
+ Lexer L(SM.getLocForStartOfFile(FileAndOffset.first), Context.getLangOpts(),
+ SrcBuffer.data(), SrcBuffer.data() + FileAndOffset.second,
+ SrcBuffer.data() + SrcBuffer.size());
+ IdentifierTable Identifiers(Context.getLangOpts());
+ clang::Token T;
+ while (!L.LexFromRawLexer(T) && L.getCurrentBufferOffset() < EndOffset) {
+ if (T.getKind() == tok::hash) {
+ L.LexFromRawLexer(T);
+ if (T.getKind() == tok::raw_identifier) {
+ clang::IdentifierInfo &II = Identifiers.get(T.getRawIdentifier());
+ if (II.getPPKeywordID() != clang::tok::pp_not_keyword)
+ return false;
+ }
+ }
+ }
+
+ return true;
+}
+
/// Calculates the new order of fields.
///
/// \returns empty vector if the list of fields doesn't match the definition.
@@ -341,6 +399,8 @@ class ReorderingConsumer : public ASTConsumer {
const RecordDecl *RD = findDefinition(RecordName, Context);
if (!RD)
return;
+ if (!isSafeToRewrite(RD, Context))
+ return;
SmallVector<unsigned, 4> NewFieldsOrder =
getNewFieldsOrder(RD, DesiredFieldsOrder);
if (NewFieldsOrder.empty())
diff --git
a/clang-tools-extra/test/clang-reorder-fields/MacroExpandsToMultipleFields.cpp
b/clang-tools-extra/test/clang-reorder-fields/MacroExpandsToMultipleFields.cpp
new file mode 100644
index 0000000000000..5bafcd19ea829
--- /dev/null
+++
b/clang-tools-extra/test/clang-reorder-fields/MacroExpandsToMultipleFields.cpp
@@ -0,0 +1,13 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order z,y,x %s --
| FileCheck %s
+
+namespace bar {
+
+#define FIELDS_DECL int x; int y; // CHECK: {{^#define FIELDS_DECL int x; int
y;}}
+
+// The order of fields should not change.
+struct Foo {
+ FIELDS_DECL // CHECK: {{^ FIELDS_DECL}}
+ int z; // CHECK-NEXT: {{^ int z;}}
+};
+
+} // end namespace bar
diff --git
a/clang-tools-extra/test/clang-reorder-fields/MultipleFieldDeclsInStatement.cpp
b/clang-tools-extra/test/clang-reorder-fields/MultipleFieldDeclsInStatement.cpp
new file mode 100644
index 0000000000000..437e7b91e27a3
--- /dev/null
+++
b/clang-tools-extra/test/clang-reorder-fields/MultipleFieldDeclsInStatement.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order z,y,x %s --
| FileCheck %s
+
+namespace bar {
+
+// The order of fields should not change.
+struct Foo {
+ int x, y; // CHECK: {{^ int x, y;}}
+ double z; // CHECK-NEXT: {{^ double z;}}
+};
+
+} // end namespace bar
diff --git
a/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundDefinition.cpp
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundDefinition.cpp
new file mode 100644
index 0000000000000..f00b4b0b57bf7
--- /dev/null
+++
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundDefinition.cpp
@@ -0,0 +1,15 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order y,x %s -- |
FileCheck %s
+
+namespace bar {
+
+#define DEFINE_FOO
+
+// This is okay to reorder.
+#ifdef DEFINE_FOO
+struct Foo {
+ int x; // CHECK: {{^ int y;}}
+ int y; // CHECK-NEXT: {{^ int x;}}
+};
+#endif
+
+} // end namespace bar
diff --git
a/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
new file mode 100644
index 0000000000000..0c22f19985a8e
--- /dev/null
+++
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
@@ -0,0 +1,15 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order y,x %s -- |
FileCheck %s
+
+namespace bar {
+
+#define DEFINE_FIELDS
+
+// This is okay to reorder.
+struct Foo {
+#ifdef DEFINE_FIELDS // CHECK: {{^#ifdef DEFINE_FIELDS}}
+ int y; // CHECK-NEXT: {{^ int y;}}
+ int x; // CHECK-NEXT: {{^ int x;}}
+#endif // CHECK-NEXT: {{^#endif}}
+};
+
+} // end namespace bar
diff --git
a/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveInDefinition.cpp
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveInDefinition.cpp
new file mode 100644
index 0000000000000..fee6b0e637b9b
--- /dev/null
+++
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveInDefinition.cpp
@@ -0,0 +1,16 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order z,y,x %s --
| FileCheck %s
+
+namespace bar {
+
+#define ADD_Z
+
+// The order of fields should not change.
+struct Foo {
+ int x; // CHECK: {{^ int x;}}
+ int y; // CHECK-NEXT: {{^ int y;}}
+#ifdef ADD_Z // CHECK-NEXT: {{^#ifdef ADD_Z}}
+ int z; // CHECK-NEXT: {{^ int z;}}
+#endif // CHECK-NEXT: {{^#endif}}
+};
+
+} // end namespace bar
>From e7bff07c40b5525199e5e288a55c55e2a875776c Mon Sep 17 00:00:00 2001
From: Vladimir Vuksanovic <vladimir.vuksano...@htecgroup.com>
Date: Mon, 16 Jun 2025 03:54:11 -0700
Subject: [PATCH 2/3] fixup! [clang-reorder-fields] Prevent rewriting
unsupported cases
---
.../PreprocessorDirectiveAroundFields.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
index 0c22f19985a8e..c37546a05afd3 100644
---
a/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
+++
b/clang-tools-extra/test/clang-reorder-fields/PreprocessorDirectiveAroundFields.cpp
@@ -7,8 +7,8 @@ namespace bar {
// This is okay to reorder.
struct Foo {
#ifdef DEFINE_FIELDS // CHECK: {{^#ifdef DEFINE_FIELDS}}
- int y; // CHECK-NEXT: {{^ int y;}}
- int x; // CHECK-NEXT: {{^ int x;}}
+ int x; // CHECK-NEXT: {{^ int y;}}
+ int y; // CHECK-NEXT: {{^ int x;}}
#endif // CHECK-NEXT: {{^#endif}}
};
>From 6873c7f8d08b7d9222fee5d520888b68741c7de9 Mon Sep 17 00:00:00 2001
From: Vladimir Vuksanovic <vladimir.vuksano...@htecgroup.com>
Date: Mon, 16 Jun 2025 07:22:26 -0700
Subject: [PATCH 3/3] fixup! [clang-reorder-fields] Prevent rewriting
unsupported cases
---
clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index 25ee1ec815a9f..d9c55fd9c0cac 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -83,7 +83,7 @@ static bool isSafeToRewrite(const RecordDecl *Decl, const
ASTContext &Context) {
// start of the first field and the end of last field.
const SourceManager &SM = Context.getSourceManager();
std::pair<FileID, unsigned> FileAndOffset =
- SM.getDecomposedLoc(Decl->field_begin()->getBeginLoc());
+ SM.getDecomposedLoc(Decl->field_begin()->getBeginLoc());
auto LastField = Decl->field_begin();
while (std::next(LastField) != Decl->field_end())
++LastField;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
