[clang-tools-extra] [clang-reorder-fields] Use expanded location for macros (PR #142147)
https://github.com/vvuksanovic created
https://github.com/llvm/llvm-project/pull/142147
Fixes macros being replaced instead of their expansion.
Closes #52632
>From 82f35e120d075eb0f55d02ff2a4d750e6e352475 Mon Sep 17 00:00:00 2001
From: Vladimir Vuksanovic
Date: Fri, 30 May 2025 05:41:09 -0700
Subject: [PATCH] [clang-reorder-fields] Use expanded location for macros
Fixes macros being replaced instead of their expansion.
Closes #52632
---
.../ReorderFieldsAction.cpp | 4
.../MacroExpansionField.cpp | 24 +++
2 files changed, 28 insertions(+)
create mode 100644
clang-tools-extra/test/clang-reorder-fields/MacroExpansionField.cpp
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index ea0207619fb2b..3b1cd18d80346 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -86,6 +86,10 @@ getNewFieldsOrder(const RecordDecl *Definition,
static void
addReplacement(SourceRange Old, SourceRange New, const ASTContext &Context,
std::map &Replacements) {
+ if (Old.getBegin().isMacroID())
+Old = Context.getSourceManager().getExpansionRange(Old).getAsRange();
+ if (New.getBegin().isMacroID())
+New = Context.getSourceManager().getExpansionRange(New).getAsRange();
StringRef NewText =
Lexer::getSourceText(CharSourceRange::getTokenRange(New),
Context.getSourceManager(), Context.getLangOpts());
diff --git
a/clang-tools-extra/test/clang-reorder-fields/MacroExpansionField.cpp
b/clang-tools-extra/test/clang-reorder-fields/MacroExpansionField.cpp
new file mode 100644
index 0..a4c3cbc1e12f4
--- /dev/null
+++ b/clang-tools-extra/test/clang-reorder-fields/MacroExpansionField.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order z,y,x %s --
| FileCheck %s
+
+namespace bar {
+
+#define INT_DECL(NAME) int NAME // CHECK: {{^#define INT_DECL\(NAME\) int
NAME}}
+#define MACRO_DECL int x; // CHECK-NEXT: {{^#define MACRO_DECL int x;}}
+
+struct Foo {
+ MACRO_DECL // CHECK: {{^ INT_DECL\(z\);}}
+ int y; // CHECK-NEXT: {{^ int y;}}
+ INT_DECL(z); // CHECK-NEXT: {{^ MACRO_DECL}}
+};
+
+#define FOO 0 // CHECK: {{^#define FOO 0}}
+#define BAR 1 // CHECK-NEXT: {{^#define BAR 1}}
+#define BAZ 2 // CHECK-NEXT: {{^#define BAZ 2}}
+
+struct Foo foo = {
+ FOO, // CHECK: {{^ BAZ,}}
+ BAR, // CHECK-NEXT: {{^ BAR,}}
+ BAZ, // CHECK-NEXT: {{^ FOO,}}
+};
+
+} // end namespace bar
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-reorder-fields] Support designated initializers (PR #142150)
https://github.com/vvuksanovic created
https://github.com/llvm/llvm-project/pull/142150
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 }
```
>From 67febc3e5b388d38025a87e7824f5a0e5d6adaaa Mon Sep 17 00:00:00 2001
From: Vladimir Vuksanovic
Date: Fri, 30 May 2025 05:37:06 -0700
Subject: [PATCH] [clang-reorder-fields] Support designated initializers
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 }
```
---
.../clang-reorder-fields/CMakeLists.txt | 1 +
.../ReorderFieldsAction.cpp | 301 +++---
.../clang-reorder-fields/utils/Designator.cpp | 256 +++
.../clang-reorder-fields/utils/Designator.h | 118 +++
.../DesignatedInitializerList.c | 31 ++
5 files changed, 659 insertions(+), 48 deletions(-)
create mode 100644 clang-tools-extra/clang-reorder-fields/utils/Designator.cpp
create mode 100644 clang-tools-extra/clang-reorder-fields/utils/Designator.h
create mode 100644
clang-tools-extra/test/clang-reorder-fields/DesignatedInitializerList.c
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
namespace clang {
@@ -81,7 +83,44 @@ getNewFieldsOrder(const RecordDecl *Definition,
return NewFieldsOrder;
}
+struct ReorderedStruct {
+public:
+ ReorderedStruct(const RecordDecl *Decl, ArrayRef NewFieldsOrder)
+ : Definition(Decl), NewFieldsOrder(NewFieldsOrder),
+NewFieldsPositions(NewFieldsOrder.size()) {
+for (unsigned I = 0; I < NewFieldsPositions.size(); ++I)
+ NewFieldsPositions[NewFieldsOrder[I]] = I;
+ }
+
+ const RecordDecl *Definition;
+ ArrayRef NewFieldsOrder;
+ SmallVector 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 &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 &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(),
- CharSourceRang
[clang-tools-extra] [clang-reorder-fields] Prevent rewriting unsupported cases (PR #142149)
https://github.com/vvuksanovic created
https://github.com/llvm/llvm-project/pull/142149
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
>From 7b4b942c8692864671ed2c51af107e5feffa600b Mon Sep 17 00:00:00 2001
From: Vladimir Vuksanovic
Date: Fri, 30 May 2025 05:42:55 -0700
Subject: [PATCH] [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 | 51 +++
.../MacroExpandsToMultipleFields.cpp | 13 +
.../MultipleFieldDeclsInStatement.cpp | 11
.../PreprocessorDirectiveInDefinition.cpp | 16 ++
4 files changed, 91 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/PreprocessorDirectiveInDefinition.cpp
diff --git a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
index ea0207619fb2b..245da5e3433c5 100644
--- a/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
+++ b/clang-tools-extra/clang-reorder-fields/ReorderFieldsAction.cpp
@@ -50,6 +50,55 @@ static const RecordDecl *findDefinition(StringRef RecordName,
return selectFirst("recordDecl", Results);
}
+static bool isSafeToRewrite(const RecordDecl *Decl, const ASTContext &Context)
{
+ // 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;
+ }
+
+ // Skip if there are preprocessor directives present.
+ const SourceManager &SM = Context.getSourceManager();
+ std::pair FileAndOffset =
+ SM.getDecomposedLoc(Decl->getSourceRange().getBegin());
+ unsigned EndOffset = SM.getFileOffset(Decl->getSourceRange().getEnd());
+ 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 +390,8 @@ class ReorderingConsumer : public ASTConsumer {
const RecordDecl *RD = findDefinition(RecordName, Context);
if (!RD)
return;
+if (!isSafeToRewrite(RD, Context))
+ return;
SmallVector 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 0..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
[clang-tools-extra] [clang-reorder-fields] Use expanded location for macros (PR #142147)
vvuksanovic wrote: @alexander-shaposhnikov https://github.com/llvm/llvm-project/pull/142147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-reorder-fields] Prevent rewriting unsupported cases (PR #142149)
vvuksanovic wrote: @alexander-shaposhnikov https://github.com/llvm/llvm-project/pull/142149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-reorder-fields] Prevent rewriting unsupported cases (PR #142149)
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
Date: Fri, 30 May 2025 05:42:55 -0700
Subject: [PATCH] [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", 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 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 NewFieldsOrder =
getNewFieldsOrder(RD, DesiredFieldsOrder);
if (NewFieldsOrder.empty())
diff --git
a/clang-tools-extra/test/clang-reorder-fields/MacroExpandsToMulti
[clang-tools-extra] [clang-reorder-fields] Support designated initializers (PR #142150)
vvuksanovic wrote: @legrosbuffle https://github.com/llvm/llvm-project/pull/142150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-reorder-fields] Prevent rewriting unsupported cases (PR #142149)
@@ -50,6 +50,55 @@ static const RecordDecl *findDefinition(StringRef RecordName,
return selectFirst("recordDecl", Results);
}
+static bool isSafeToRewrite(const RecordDecl *Decl, const ASTContext &Context)
{
+ // 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;
+ }
+
+ // Skip if there are preprocessor directives present.
+ const SourceManager &SM = Context.getSourceManager();
+ std::pair FileAndOffset =
+ SM.getDecomposedLoc(Decl->getSourceRange().getBegin());
+ unsigned EndOffset = SM.getFileOffset(Decl->getSourceRange().getEnd());
+ StringRef SrcBuffer = SM.getBufferData(FileAndOffset.first);
+ Lexer L(SM.getLocForStartOfFile(FileAndOffset.first), Context.getLangOpts(),
+ SrcBuffer.data(), SrcBuffer.data() + FileAndOffset.second,
vvuksanovic wrote:
I agree. Changed the range and added a test.
https://github.com/llvm/llvm-project/pull/142149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-reorder-fields] Prevent rewriting unsupported cases (PR #142149)
@@ -0,0 +1,16 @@ +// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order z,y,x %s -- | FileCheck %s vvuksanovic wrote: Done. https://github.com/llvm/llvm-project/pull/142149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
