hokein created this revision.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.
- --new_depend_on_old: new header will include old header
- --old_depend_on_new: old header will include new header.
https://reviews.llvm.org/D26966
Files:
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/tool/ClangMoveMain.cpp
unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -419,6 +419,45 @@
EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
}
+TEST(ClangMove, AddDependentNewHeader) {
+ const std::string TestHeader = "class A {\npublic:\n int f();\n};\n"
+ "class B {};\n";
+ const std::string TestCode = "#include \"foo.h\"\n";
+ const std::string ExpectedTestHeader = "#include \"new_foo.h\"\nclass B {};\n";
+ move::ClangMoveTool::MoveDefinitionSpec Spec;
+ Spec.Names.push_back("A");
+ Spec.OldHeader = "foo.h";
+ Spec.OldCC = "foo.cc";
+ Spec.NewHeader = "new_foo.h";
+ Spec.NewCC = "new_foo.cc";
+ Spec.OldDependOnNew = true;
+ auto Results = runClangMoveOnCode(Spec, TestHeader.c_str(), TestCode.c_str());
+ EXPECT_EQ(ExpectedTestHeader, Results[Spec.OldHeader]);
+}
+
+TEST(ClangMove, AddDependentOldHeader) {
+ const char TestHeader[] = "class A {\npublic:\n int f();\n};\n"
+ "class B {};\n";
+ const char TestCode[] = "#include \"foo.h\"\n";
+ const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n"
+ "#define NEW_FOO_H\n"
+ "\n"
+ "#include \"foo.h\"\n"
+ "\n"
+ "class B {};\n"
+ "\n"
+ "#endif // NEW_FOO_H\n";
+ move::ClangMoveTool::MoveDefinitionSpec Spec;
+ Spec.Names.push_back("B");
+ Spec.OldHeader = "foo.h";
+ Spec.OldCC = "foo.cc";
+ Spec.NewHeader = "new_foo.h";
+ Spec.NewCC = "new_foo.cc";
+ Spec.NewDependOnOld = true;
+ auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+ EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+}
+
} // namespace
} // namespce move
} // namespace clang
Index: clang-move/tool/ClangMoveMain.cpp
===================================================================
--- clang-move/tool/ClangMoveMain.cpp
+++ clang-move/tool/ClangMoveMain.cpp
@@ -61,6 +61,20 @@
NewCC("new_cc", cl::desc("The relative/absolute file path of new cc."),
cl::cat(ClangMoveCategory));
+cl::opt<bool> OldDependOnNew(
+ "old_depend_on_new",
+ cl::desc(
+ "Whether old header depends on new header. If true, clan-move will "
+ "add #include of new header to old header."),
+ cl::init(false), cl::cat(ClangMoveCategory));
+
+cl::opt<bool> NewDependOnOld(
+ "new_depend_on_old",
+ cl::desc(
+ "Whether new header depends on old header. If true, clan-move will "
+ "add #include of old header to new header."),
+ cl::init(false), cl::cat(ClangMoveCategory));
+
cl::opt<std::string>
Style("style",
cl::desc("The style name used for reformatting. Default is \"llvm\""),
@@ -95,6 +109,8 @@
Spec.NewHeader = NewHeader;
Spec.OldCC = OldCC;
Spec.NewCC = NewCC;
+ Spec.OldDependOnNew = OldDependOnNew;
+ Spec.NewDependOnOld = NewDependOnOld;
llvm::SmallString<128> InitialDirectory;
if (std::error_code EC = llvm::sys::fs::current_path(InitialDirectory))
Index: clang-move/ClangMove.h
===================================================================
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -15,6 +15,7 @@
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/StringMap.h"
#include <map>
#include <memory>
#include <string>
@@ -52,6 +53,12 @@
std::string NewHeader;
// The file path of new cc, can be relative path and absolute path.
std::string NewCC;
+ // Whether old.h depends on new.h. If true, additional #include "new.h" will
+ // be added in old.h.
+ bool OldDependOnNew = false;
+ // Whether new.h depends on old.h. If true, additional #include "old.h" will
+ // be added in new.h.
+ bool NewDependOnOld = false;
};
ClangMoveTool(
@@ -83,7 +90,7 @@
std::vector<MovedDecl> &getMovedDecls() { return MovedDecls; }
- std::vector<MovedDecl> &getRemovedDecls() { return RemovedDecls; }
+ void addRemovedDecl(const MovedDecl &Decl);
llvm::SmallPtrSet<const NamedDecl *, 8> &getUnremovedDeclsInOldHeader() {
return UnremovedDeclsInOldHeader;
@@ -127,6 +134,8 @@
/// #include "old.h") in old.cc, including the enclosing quotes or angle
/// brackets.
clang::CharSourceRange OldHeaderIncludeRange;
+ /// Mapping from FilePath to FileID.
+ llvm::StringMap<FileID> FilePathToFileID;
};
class ClangMoveAction : public clang::ASTFrontendAction {
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -150,7 +150,7 @@
MoveTool->getMovedDecls().emplace_back(D,
&Result.Context->getSourceManager());
MoveTool->getUnremovedDeclsInOldHeader().erase(D);
- MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+ MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
}
private:
@@ -181,7 +181,7 @@
// case.
if (!CMD->isInlined()) {
MoveTool->getMovedDecls().emplace_back(CMD, SM);
- MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+ MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
// Get template class method from its method declaration as
// UnremovedDecls stores template class method.
if (const auto *FTD = CMD->getDescribedFunctionTemplate())
@@ -194,7 +194,7 @@
void MatchClassStaticVariable(const clang::NamedDecl *VD,
clang::SourceManager* SM) {
MoveTool->getMovedDecls().emplace_back(VD, SM);
- MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+ MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
MoveTool->getUnremovedDeclsInOldHeader().erase(VD);
}
@@ -206,7 +206,7 @@
MoveTool->getMovedDecls().emplace_back(TC, SM);
else
MoveTool->getMovedDecls().emplace_back(CD, SM);
- MoveTool->getRemovedDecls().push_back(MoveTool->getMovedDecls().back());
+ MoveTool->addRemovedDecl(MoveTool->getMovedDecls().back());
MoveTool->getUnremovedDeclsInOldHeader().erase(
MoveTool->getMovedDecls().back().Decl);
}
@@ -305,7 +305,8 @@
clang::tooling::Replacements
createInsertedReplacements(const std::vector<std::string> &Includes,
const std::vector<ClangMoveTool::MovedDecl> &Decls,
- llvm::StringRef FileName, bool IsHeader = false) {
+ llvm::StringRef FileName, bool IsHeader = false,
+ StringRef OldHeaderInclude = "") {
std::string NewCode;
std::string GuardName(FileName);
if (IsHeader) {
@@ -318,6 +319,7 @@
NewCode += "#define " + GuardName + "\n\n";
}
+ NewCode += OldHeaderInclude;
// Add #Includes.
for (const auto &Include : Includes)
NewCode += Include;
@@ -410,7 +412,21 @@
CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
}
+void ClangMoveTool::addRemovedDecl(const MovedDecl &Decl) {
+ const auto &SM = *Decl.SM;
+ auto Loc = Decl.Decl->getLocation();
+ StringRef FilePath = SM.getFilename(Loc);
+ FilePathToFileID[FilePath] = SM.getFileID(Loc);
+ RemovedDecls.push_back(Decl);
+}
+
void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
+ if (Spec.OldDependOnNew && Spec.NewDependOnOld) {
+ llvm::errs() << "Provide either --old_depend_on_new or "
+ "--new_depend_on_old. clang-move doesn't support these two "
+ "options at same time (It will introduce include cycle).\n";
+ return;
+ }
Optional<ast_matchers::internal::Matcher<NamedDecl>> HasAnySymbolNames;
for (StringRef SymbolName: Spec.Names) {
llvm::StringRef GlobalSymbolName = SymbolName.trim().ltrim(':');
@@ -575,32 +591,48 @@
}
void ClangMoveTool::removeClassDefinitionInOldFiles() {
+ const SourceManager* SM = nullptr;
for (const auto &MovedDecl : RemovedDecls) {
- const auto &SM = *MovedDecl.SM;
- auto Range = GetFullRange(&SM, MovedDecl.Decl);
+ SM = MovedDecl.SM;
+ auto Range = GetFullRange(SM, MovedDecl.Decl);
clang::tooling::Replacement RemoveReplacement(
- *MovedDecl.SM,
+ *SM,
clang::CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()),
"");
std::string FilePath = RemoveReplacement.getFilePath().str();
auto Err = FileToReplacements[FilePath].add(RemoveReplacement);
- if (Err) {
+ if (Err)
llvm::errs() << llvm::toString(std::move(Err)) << "\n";
- continue;
- }
-
- llvm::StringRef Code =
- SM.getBufferData(SM.getFileID(MovedDecl.Decl->getLocation()));
- format::FormatStyle Style =
- format::getStyle("file", FilePath, FallbackStyle);
- auto CleanReplacements = format::cleanupAroundReplacements(
- Code, FileToReplacements[FilePath], Style);
+ }
- if (!CleanReplacements) {
- llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
- continue;
+ if (SM) {
+ for (auto& It: FileToReplacements) {
+ StringRef FilePath = It.first;
+ if (Spec.OldDependOnNew &&
+ MakeAbsolutePath(*SM, FilePath) == makeAbsolutePath(Spec.OldHeader)) {
+ std::string IncludeNewH = "#include \"" + Spec.NewHeader + "\"\n";
+ auto Err = It.second.add(
+ tooling::Replacement(FilePath, UINT_MAX, 0, IncludeNewH));
+ if (Err)
+ llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+ }
+
+ auto SI = FilePathToFileID.find(FilePath);
+ if (SI == FilePathToFileID.end())
+ continue;
+ FileID ID = SI->second;
+ llvm::StringRef Code = SM->getBufferData(ID);
+ format::FormatStyle Style =
+ format::getStyle("file", FilePath, FallbackStyle);
+ auto CleanReplacements = format::cleanupAroundReplacements(
+ Code, FileToReplacements[FilePath], Style);
+
+ if (!CleanReplacements) {
+ llvm::errs() << llvm::toString(CleanReplacements.takeError()) << "\n";
+ continue;
+ }
+ FileToReplacements[FilePath] = *CleanReplacements;
}
- FileToReplacements[FilePath] = *CleanReplacements;
}
}
@@ -615,9 +647,12 @@
NewCCDecls.push_back(MovedDecl);
}
- if (!Spec.NewHeader.empty())
+ if (!Spec.NewHeader.empty()) {
+ std::string OldHeaderInclude = "#include \"" + Spec.OldHeader + "\"\n";
FileToReplacements[Spec.NewHeader] = createInsertedReplacements(
- HeaderIncludes, NewHeaderDecls, Spec.NewHeader, /*IsHeader=*/true);
+ HeaderIncludes, NewHeaderDecls, Spec.NewHeader, /*IsHeader=*/true,
+ Spec.NewDependOnOld ? OldHeaderInclude : "");
+ }
if (!Spec.NewCC.empty())
FileToReplacements[Spec.NewCC] =
createInsertedReplacements(CCIncludes, NewCCDecls, Spec.NewCC);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
