alexshap updated this revision to Diff 69445.
alexshap added a comment.
Address @djasper 's comments.
1. Try to flash out comments for helper functions / various interesting places
in my code
2. More elaborate error-handling
3. Adjust the names of the variables: I -> i, E -> e
4. Minor cleanup & clang-format -i
Repository:
rL LLVM
https://reviews.llvm.org/D23279
Files:
CMakeLists.txt
clang-reorder-fields/CMakeLists.txt
clang-reorder-fields/ReorderFieldsAction.cpp
clang-reorder-fields/ReorderFieldsAction.h
clang-reorder-fields/tool/CMakeLists.txt
clang-reorder-fields/tool/ClangReorderFields.cpp
test/CMakeLists.txt
test/clang-reorder-fields/CStructAmbiguousName.cpp
test/clang-reorder-fields/CStructFieldsOrder.cpp
test/clang-reorder-fields/ClassMixedInitialization.cpp
test/clang-reorder-fields/ClassSimpleCtor.cpp
Index: test/clang-reorder-fields/ClassSimpleCtor.cpp
===================================================================
--- test/clang-reorder-fields/ClassSimpleCtor.cpp
+++ test/clang-reorder-fields/ClassSimpleCtor.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order s1,x,z,s2 %s -- | FileCheck %s
+
+class Foo {
+public:
+ Foo();
+
+private:
+ int x; // CHECK: {{^ const char \*s1;}}
+ const char *s1; // CHECK-NEXT: {{^ int x;}}
+ const char *s2; // CHECK-NEXT: {{^ double z;}}
+ double z; // CHECK-NEXT: {{^ const char \*s2;}}
+};
+
+Foo::Foo():
+ x(12), // CHECK: {{^ s1\("abc"\),}}
+ s1("abc"), // CHECK-NEXT: {{^ x\(12\),}}
+ s2("def"), // CHECK-NEXT: {{^ z\(3.14\),}}
+ z(3.14) // CHECK-NEXT: {{^ s2\("def"\)}}
+{}
+
+int main() {
+ Foo foo;
+ return 0;
+}
Index: test/clang-reorder-fields/ClassMixedInitialization.cpp
===================================================================
--- test/clang-reorder-fields/ClassMixedInitialization.cpp
+++ test/clang-reorder-fields/ClassMixedInitialization.cpp
@@ -0,0 +1,24 @@
+// RUN: clang-reorder-fields -record-name Foo -fields-order e,x,pi,s2,s1 %s -- -std=c++11 | FileCheck %s
+
+class Foo {
+public:
+ Foo();
+
+private:
+ int x; // CHECK: {{^ double e = 2.71;}}
+ const char *s1; // CHECK-NEXT: {{^ int x;}}
+ const char *s2; // CHECK-NEXT: {{^ double pi = 3.14;}}
+ double pi = 3.14; // CHECK-NEXT: {{^ const char \*s2;}}
+ double e = 2.71; // CHECK-NEXT: {{^ const char \*s1;}}
+};
+
+Foo::Foo():
+ x(12), // CHECK: {{^ x\(12\)}},
+ s1("abc"), // CHECK-NEXT: {{^ s2\("def"\)}},
+ s2("def") // CHECK-NEXT: {{^ s1\("abc"\)}}
+{}
+
+int main() {
+ Foo foo;
+ return 0;
+}
Index: test/clang-reorder-fields/CStructFieldsOrder.cpp
===================================================================
--- test/clang-reorder-fields/CStructFieldsOrder.cpp
+++ test/clang-reorder-fields/CStructFieldsOrder.cpp
@@ -0,0 +1,16 @@
+// RUN: clang-reorder-fields -record-name ::bar::Foo -fields-order z,w,y,x %s -- | FileCheck %s
+
+namespace bar {
+struct Foo {
+ const int* x; // CHECK: {{^ double z;}}
+ int y; // CHECK-NEXT: {{^ int w;}}
+ double z; // CHECK-NEXT: {{^ int y;}}
+ int w; // CHECK-NEXT: {{^ const int\* x}}
+};
+} // end namespace bar
+
+int main() {
+ const int x = 13;
+ bar::Foo foo = { &x, 0, 1.29, 17 }; // CHECK: {{^ bar::Foo foo = { 1.29, 17, 0, &x };}}
+ return 0;
+}
Index: test/clang-reorder-fields/CStructAmbiguousName.cpp
===================================================================
--- test/clang-reorder-fields/CStructAmbiguousName.cpp
+++ test/clang-reorder-fields/CStructAmbiguousName.cpp
@@ -0,0 +1,18 @@
+// RUN: clang-reorder-fields -record-name ::Foo -fields-order y,x %s -- | FileCheck %s
+
+struct Foo {
+ int x; // CHECK: {{^ double y;}}
+ double y; // CHECK-NEXT: {{^ int x;}}
+};
+
+namespace bar {
+struct Foo {
+ int x; // CHECK: {{^ int x;}}
+ double y; // CHECK-NEXT: {{^ double y;}}
+};
+} // end namespace bar
+
+int main() {
+ bar::Foo foo = { 1, 1.7 }; // CHECK: {{^ bar::Foo foo = { 1, 1.7 };}}
+ return 0;
+}
Index: test/CMakeLists.txt
===================================================================
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -45,6 +45,7 @@
clang-include-fixer
clang-query
clang-rename
+ clang-reorder-fields
clang-tidy
find-all-symbols
modularize
Index: clang-reorder-fields/tool/ClangReorderFields.cpp
===================================================================
--- clang-reorder-fields/tool/ClangReorderFields.cpp
+++ clang-reorder-fields/tool/ClangReorderFields.cpp
@@ -0,0 +1,88 @@
+//===-- tools/extra/clang-reorder-fields/tool/ClangReorderFields.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the implementation of clang-reorder-fields tool
+///
+//===----------------------------------------------------------------------===//
+
+#include "../ReorderFieldsAction.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include <cstdlib>
+#include <string>
+#include <system_error>
+
+using namespace llvm;
+using namespace clang;
+
+cl::OptionCategory ClangReorderFieldsCategory("clang-reorder-fields options");
+
+static cl::opt<std::string>
+ RecordName("record-name", cl::Required,
+ cl::desc("The name of the struct/class."),
+ cl::cat(ClangReorderFieldsCategory));
+
+static cl::list<std::string> FieldsOrder("fields-order", cl::CommaSeparated,
+ cl::OneOrMore,
+ cl::desc("The desired fields order."),
+ cl::cat(ClangReorderFieldsCategory));
+
+static cl::opt<bool> Inplace("i", cl::desc("Overwrite edited files."),
+ cl::cat(ClangReorderFieldsCategory));
+
+const char Usage[] = "A tool to reorder fields in C/C++ structs/classes.\n";
+
+int main(int argc, const char **argv) {
+ tooling::CommonOptionsParser OP(argc, argv, ClangReorderFieldsCategory,
+ Usage);
+
+ auto Files = OP.getSourcePathList();
+ tooling::RefactoringTool Tool(OP.getCompilations(), Files);
+
+ reorder_fields::ReorderFieldsAction Action(RecordName, FieldsOrder,
+ Tool.getReplacements());
+
+ auto Factory = tooling::newFrontendActionFactory(&Action);
+
+ if (Inplace)
+ return Tool.runAndSave(Factory.get());
+
+ int ExitCode = Tool.run(Factory.get());
+ LangOptions DefaultLangOptions;
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts(new DiagnosticOptions());
+ TextDiagnosticPrinter DiagnosticPrinter(errs(), &*DiagOpts);
+ DiagnosticsEngine Diagnostics(
+ IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs()), &*DiagOpts,
+ &DiagnosticPrinter, false);
+
+ auto &FileMgr = Tool.getFiles();
+ SourceManager Sources(Diagnostics, FileMgr);
+ Rewriter Rewrite(Sources, DefaultLangOptions);
+ Tool.applyAllReplacements(Rewrite);
+
+ for (const auto &File : Files) {
+ const auto *Entry = FileMgr.getFile(File);
+ const auto ID = Sources.translateFile(Entry);
+ Rewrite.getEditBuffer(ID).write(outs());
+ }
+
+ return ExitCode;
+}
Index: clang-reorder-fields/tool/CMakeLists.txt
===================================================================
--- clang-reorder-fields/tool/CMakeLists.txt
+++ clang-reorder-fields/tool/CMakeLists.txt
@@ -0,0 +1,12 @@
+add_clang_executable(clang-reorder-fields ClangReorderFields.cpp)
+
+target_link_libraries(clang-reorder-fields
+ clangBasic
+ clangFrontend
+ clangReorderFields
+ clangRewrite
+ clangTooling
+ clangToolingCore
+ )
+
+install(TARGETS clang-reorder-fields RUNTIME DESTINATION bin)
Index: clang-reorder-fields/ReorderFieldsAction.h
===================================================================
--- clang-reorder-fields/ReorderFieldsAction.h
+++ clang-reorder-fields/ReorderFieldsAction.h
@@ -0,0 +1,47 @@
+//===-- tools/extra/clang-reorder-fields/ReorderFieldsAction.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the declarations of the ReorderFieldsAction class and
+/// the FieldPosition struct.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_REORDER_FIELDS_ACTION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_REORDER_FIELDS_ACTION_H
+
+#include "clang/Tooling/Refactoring.h"
+
+namespace clang {
+class ASTConsumer;
+
+namespace reorder_fields {
+
+class ReorderFieldsAction {
+ llvm::StringRef RecordName;
+ llvm::ArrayRef<std::string> DesiredFieldsOrder;
+ std::map<std::string, tooling::Replacements> &Replacements;
+
+public:
+ ReorderFieldsAction(
+ llvm::StringRef RecordName,
+ llvm::ArrayRef<std::string> DesiredFieldsOrder,
+ std::map<std::string, tooling::Replacements> &Replacements)
+ : RecordName(RecordName), DesiredFieldsOrder(DesiredFieldsOrder),
+ Replacements(Replacements) {}
+
+ ReorderFieldsAction(const ReorderFieldsAction &) = delete;
+ ReorderFieldsAction &operator=(const ReorderFieldsAction &) = delete;
+
+ std::unique_ptr<ASTConsumer> newASTConsumer();
+};
+} // namespace reorder_fields
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_REORDER_FIELDS_ACTION_H
Index: clang-reorder-fields/ReorderFieldsAction.cpp
===================================================================
--- clang-reorder-fields/ReorderFieldsAction.cpp
+++ clang-reorder-fields/ReorderFieldsAction.cpp
@@ -0,0 +1,252 @@
+//===-- tools/extra/clang-reorder-fields/ReorderFieldsAction.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file contains the definition of the
+/// ReorderFieldsAction::newASTConsumer method
+///
+//===----------------------------------------------------------------------===//
+
+#include "ReorderFieldsAction.h"
+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Refactoring.h"
+#include <algorithm>
+#include <string>
+
+namespace clang {
+using namespace ast_matchers;
+
+namespace reorder_fields {
+
+/// \brief Finds the definition of a record by name.
+///
+/// \returns nullptr if the name is ambiguous or not found.
+static const CXXRecordDecl *findDefinition(StringRef RecordName,
+ ASTContext &Context) {
+ auto Results = match(
+ recordDecl(hasName(RecordName), isDefinition()).bind("cxxRecordDecl"),
+ Context);
+ if (Results.empty()) {
+ llvm::errs() << "Definition of " << RecordName << " not found\n";
+ return nullptr;
+ }
+ if (Results.size() > 1) {
+ llvm::errs() << "The name " << RecordName
+ << " is ambiguous, several definitions found\n";
+ return nullptr;
+ }
+ return selectFirst<CXXRecordDecl>("cxxRecordDecl", Results);
+}
+
+/// \brief Calculates the new order of fields.
+///
+/// \returns empty vector if the list of fields doesn't match the definition.
+static SmallVector<unsigned, 4>
+getNewFieldsOrder(const CXXRecordDecl *Definition,
+ ArrayRef<std::string> DesiredFieldsOrder) {
+ assert(Definition && "Definition is null");
+
+ llvm::StringMap<unsigned> NameToIndex;
+ for (const auto *Field : Definition->fields())
+ NameToIndex[Field->getName()] = Field->getFieldIndex();
+
+ if (DesiredFieldsOrder.size() != NameToIndex.size()) {
+ llvm::errs() << "Number of provided fields doesn't match definition.\n";
+ return {};
+ }
+ SmallVector<unsigned, 4> NewFieldsOrder;
+ for (const auto &Name : DesiredFieldsOrder) {
+ if (!NameToIndex.count(Name)) {
+ llvm::errs() << "Field " << Name << " not found in definition.\n";
+ return {};
+ }
+ NewFieldsOrder.push_back(NameToIndex[Name]);
+ }
+ assert(NewFieldsOrder.size() == NameToIndex.size());
+ return NewFieldsOrder;
+}
+
+// FIXME: error-handling
+/// \brief Replaces one range of source code by another.
+static void
+addReplacement(SourceRange Old, SourceRange New, const ASTContext &Context,
+ std::map<std::string, tooling::Replacements> &Replacements) {
+ StringRef NewText =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(New),
+ Context.getSourceManager(), Context.getLangOpts());
+ tooling::Replacement R(Context.getSourceManager(),
+ CharSourceRange::getTokenRange(Old), NewText,
+ Context.getLangOpts());
+ consumeError(Replacements[R.getFilePath()].add(R));
+}
+
+/// \brief Reorders fields in the definition of a struct/class.
+///
+/// At the moment reodering of fields with
+/// different accesses (public/protected/private) is not supported.
+/// \returns true on success.
+static bool reorderFieldsInDefinition(
+ const CXXRecordDecl *Definition, ArrayRef<unsigned> NewFieldsOrder,
+ const ASTContext &Context,
+ std::map<std::string, tooling::Replacements> &Replacements) {
+ assert(Definition && "Definition is null");
+
+ SmallVector<const FieldDecl *, 10> Fields;
+ for (const auto *Field : Definition->fields())
+ Fields.push_back(Field);
+
+ // Check that the permutation of the fields doesn't change the accesses
+ for (const auto *Field : Definition->fields()) {
+ const auto FieldIndex = Field->getFieldIndex();
+ if (Field->getAccess() != Fields[NewFieldsOrder[FieldIndex]]->getAccess()) {
+ llvm::errs() << "Currently reodering of fields with different accesses "
+ "is not supported\n";
+ return false;
+ }
+ }
+
+ for (const auto *Field : Definition->fields()) {
+ const auto FieldIndex = Field->getFieldIndex();
+ if (FieldIndex == NewFieldsOrder[FieldIndex])
+ continue;
+ addReplacement(Field->getSourceRange(),
+ Fields[NewFieldsOrder[FieldIndex]]->getSourceRange(),
+ Context, Replacements);
+ }
+ return true;
+}
+
+/// \brief Reorders initializers in a C++ struct/class constructor.
+///
+/// A constructor can have initializers for an arbitrary subset of the classes
+/// fields.
+/// Thus, we need to ensure that we reorder just the initializers that a
+/// present.
+static void reorderFieldsInConstructor(
+ const CXXConstructorDecl *CtorDecl, ArrayRef<unsigned> NewFieldsOrder,
+ const ASTContext &Context,
+ std::map<std::string, tooling::Replacements> &Replacements) {
+ assert(CtorDecl && "Constructor declaration is null");
+ assert(CtorDecl->isThisDeclarationADefinition() && "Not a definition");
+
+ SmallVector<unsigned, 10> NewFieldsPositions(NewFieldsOrder.size());
+ for (unsigned i = 0, e = NewFieldsOrder.size(); i < e; ++i)
+ NewFieldsPositions[NewFieldsOrder[i]] = i;
+
+ SmallVector<const CXXCtorInitializer *, 10> OldWrittenInitializersOrder;
+ SmallVector<const CXXCtorInitializer *, 10> NewWrittenInitializersOrder;
+ for (const auto *Initializer : CtorDecl->inits()) {
+ if (!Initializer->isWritten())
+ continue;
+ OldWrittenInitializersOrder.push_back(Initializer);
+ NewWrittenInitializersOrder.push_back(Initializer);
+ }
+ auto ByFieldNewPosition = [&](const CXXCtorInitializer *LHS,
+ const CXXCtorInitializer *RHS) {
+ assert(LHS && RHS);
+ return NewFieldsPositions[LHS->getMember()->getFieldIndex()] <
+ NewFieldsPositions[RHS->getMember()->getFieldIndex()];
+ };
+ std::sort(std::begin(NewWrittenInitializersOrder),
+ std::end(NewWrittenInitializersOrder), ByFieldNewPosition);
+ assert(OldWrittenInitializersOrder.size() ==
+ NewWrittenInitializersOrder.size());
+ for (unsigned i = 0, e = NewWrittenInitializersOrder.size(); i < e; ++i)
+ if (OldWrittenInitializersOrder[i] != NewWrittenInitializersOrder[i])
+ addReplacement(OldWrittenInitializersOrder[i]->getSourceRange(),
+ NewWrittenInitializersOrder[i]->getSourceRange(), Context,
+ Replacements);
+}
+
+/// \brief Reorders initializers in the brace initialization of an aggregate.
+///
+/// At the moment partial initialization is not supported.
+/// \returns true on success
+static bool reorderFieldsInInitListExpr(
+ const InitListExpr *InitListEx, ArrayRef<unsigned> NewFieldsOrder,
+ const ASTContext &Context,
+ std::map<std::string, tooling::Replacements> &Replacements) {
+ assert(InitListEx && "Init list expression is null");
+ if (!InitListEx->isExplicit() || !InitListEx->getNumInits())
+ return true;
+ if (InitListEx->getNumInits() != NewFieldsOrder.size()) {
+ llvm::errs() << "Currently only full initialization is supported\n";
+ return false;
+ }
+ for (unsigned i = 0, e = InitListEx->getNumInits(); i < e; ++i)
+ if (i != NewFieldsOrder[i])
+ addReplacement(InitListEx->getInit(i)->getSourceRange(),
+ InitListEx->getInit(NewFieldsOrder[i])->getSourceRange(),
+ Context, Replacements);
+ return true;
+}
+
+namespace {
+class ReorderingConsumer : public ASTConsumer {
+ StringRef RecordName;
+ ArrayRef<std::string> DesiredFieldsOrder;
+ std::map<std::string, tooling::Replacements> &Replacements;
+
+public:
+ ReorderingConsumer(StringRef RecordName,
+ ArrayRef<std::string> DesiredFieldsOrder,
+ std::map<std::string, tooling::Replacements> &Replacements)
+ : RecordName(RecordName), DesiredFieldsOrder(DesiredFieldsOrder),
+ Replacements(Replacements) {}
+
+ ReorderingConsumer(const ReorderingConsumer &) = delete;
+ ReorderingConsumer &operator=(const ReorderingConsumer &) = delete;
+
+ void HandleTranslationUnit(ASTContext &Context) override {
+ const CXXRecordDecl *RD = findDefinition(RecordName, Context);
+ if (!RD)
+ return;
+ SmallVector<unsigned, 4> NewFieldsOrder =
+ getNewFieldsOrder(RD, DesiredFieldsOrder);
+ if (NewFieldsOrder.empty())
+ return;
+ if (!reorderFieldsInDefinition(RD, NewFieldsOrder, Context, Replacements))
+ return;
+ for (const auto *C : RD->ctors()) {
+ if (C->isImplicit() || C->isDelegatingConstructor())
+ continue;
+ if (const auto *D = dyn_cast<CXXConstructorDecl>(C->getDefinition()))
+ reorderFieldsInConstructor(cast<const CXXConstructorDecl>(D),
+ NewFieldsOrder, Context, Replacements);
+ }
+
+ // We only need to reorder init list expressions for aggregate types.
+ // Now (v0) partial initialization is not supported.
+ // For other types the order of constructor parameters is used,
+ // which we don't change at the moment.
+ if (!RD->isAggregate())
+ return;
+ for (auto Result :
+ match(initListExpr(hasType(equalsNode(RD))).bind("initListExpr"),
+ Context))
+ reorderFieldsInInitListExpr(
+ Result.getNodeAs<InitListExpr>("initListExpr"), NewFieldsOrder,
+ Context, Replacements);
+ }
+};
+} // end anonymous namespace
+
+std::unique_ptr<ASTConsumer> ReorderFieldsAction::newASTConsumer() {
+ return llvm::make_unique<ReorderingConsumer>(RecordName, DesiredFieldsOrder,
+ Replacements);
+}
+
+} // namespace reorder_fields
+} // namespace clang
Index: clang-reorder-fields/CMakeLists.txt
===================================================================
--- clang-reorder-fields/CMakeLists.txt
+++ clang-reorder-fields/CMakeLists.txt
@@ -0,0 +1,15 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangReorderFields
+ ReorderFieldsAction.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangIndex
+ clangLex
+ clangToolingCore
+ )
+
+add_subdirectory(tool)
Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -1,5 +1,6 @@
add_subdirectory(clang-apply-replacements)
add_subdirectory(clang-rename)
+add_subdirectory(clang-reorder-fields)
add_subdirectory(modularize)
if(CLANG_ENABLE_STATIC_ANALYZER)
add_subdirectory(clang-tidy)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
