hokein updated this revision to Diff 132111.
hokein marked an inline comment as done.
hokein added a comment.
address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42639
Files:
clang-move/ClangMove.cpp
clang-move/ClangMove.h
unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -293,6 +293,32 @@
EXPECT_EQ(0u, Results.size());
}
+TEST(ClangMove, HeaderIncludeSelf) {
+ move::MoveDefinitionSpec Spec;
+ Spec.Names = {std::string("Foo")};
+ Spec.OldHeader = "foo.h";
+ Spec.OldCC = "foo.cc";
+ Spec.NewHeader = "new_foo.h";
+ Spec.NewCC = "new_foo.cc";
+
+ const char TestHeader[] = "#ifndef FOO_H\n"
+ "#define FOO_H\n"
+ "#include \"foo.h\"\n"
+ "class Foo {};\n"
+ "#endif\n";
+ const char TestCode[] = "#include \"foo.h\"";
+ const char ExpectedNewHeader[] = "#ifndef FOO_H\n"
+ "#define FOO_H\n"
+ "#include \"new_foo.h\"\n"
+ "class Foo {};\n"
+ "#endif\n";
+ const char ExpectedNewCC[] = "#include \"new_foo.h\"";
+ auto Results = runClangMoveOnCode(Spec, TestHeader, TestCode);
+ EXPECT_EQ("", Results[Spec.OldHeader]);
+ EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+ EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+}
+
TEST(ClangMove, MoveAll) {
std::vector<std::string> TestHeaders = {
"class A {\npublic:\n int f();\n};",
Index: clang-move/ClangMove.h
===================================================================
--- clang-move/ClangMove.h
+++ clang-move/ClangMove.h
@@ -176,7 +176,11 @@
/// The source range for the written file name in #include (i.e. "old.h" for
/// #include "old.h") in old.cc, including the enclosing quotes or angle
/// brackets.
- clang::CharSourceRange OldHeaderIncludeRange;
+ clang::CharSourceRange OldHeaderIncludeRangeInCC;
+ /// The source range for the written file name in #include (i.e. "old.h" for
+ /// #include "old.h") in old.h, including the enclosing quotes or angle
+ /// brackets.
+ clang::CharSourceRange OldHeaderIncludeRangeInHeader;
/// Mapping from FilePath to FileID, which can be used in post processes like
/// cleanup around replacements.
llvm::StringMap<FileID> FilePathToFileID;
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -694,22 +694,28 @@
const SourceManager &SM) {
SmallVector<char, 128> HeaderWithSearchPath;
llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader);
- std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
- if (AbsoluteOldHeader ==
+ std::string AbsoluteIncludeHeader =
MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(),
- HeaderWithSearchPath.size()))) {
- OldHeaderIncludeRange = IncludeFilenameRange;
- return;
- }
-
+ HeaderWithSearchPath.size()));
std::string IncludeLine =
IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
: ("#include \"" + IncludeHeader + "\"\n").str();
+ std::string AbsoluteOldHeader = makeAbsolutePath(Context->Spec.OldHeader);
std::string AbsoluteCurrentFile = MakeAbsolutePath(SM, FileName);
if (AbsoluteOldHeader == AbsoluteCurrentFile) {
+ // Find old.h includes "old.h".
+ if (AbsoluteOldHeader == AbsoluteIncludeHeader) {
+ OldHeaderIncludeRangeInHeader = IncludeFilenameRange;
+ return;
+ }
HeaderIncludes.push_back(IncludeLine);
} else if (makeAbsolutePath(Context->Spec.OldCC) == AbsoluteCurrentFile) {
+ // Find old.cc includes "old.h".
+ if (AbsoluteOldHeader == AbsoluteIncludeHeader) {
+ OldHeaderIncludeRangeInCC = IncludeFilenameRange;
+ return;
+ }
CCIncludes.push_back(IncludeLine);
}
}
@@ -857,13 +863,18 @@
if (!NewFile.empty()) {
auto AllCode = clang::tooling::Replacements(
clang::tooling::Replacement(NewFile, 0, 0, Code));
- // If we are moving from old.cc, an extra step is required: excluding
- // the #include of "old.h", instead, we replace it with #include of "new.h".
- if (Context->Spec.NewCC == NewFile && OldHeaderIncludeRange.isValid()) {
- AllCode = AllCode.merge(
- clang::tooling::Replacements(clang::tooling::Replacement(
- SM, OldHeaderIncludeRange, '"' + Context->Spec.NewHeader + '"')));
- }
+ auto ReplaceOldInclude = [&](clang::CharSourceRange OldHeaderIncludeRange) {
+ AllCode = AllCode.merge(clang::tooling::Replacements(
+ clang::tooling::Replacement(SM, OldHeaderIncludeRange,
+ '"' + Context->Spec.NewHeader + '"')));
+ };
+ // Fix the case where old.h/old.cc includes "old.h", we replace the
+ // `#include "old.h"` with `#include "new.h"`.
+ if (Context->Spec.NewCC == NewFile && OldHeaderIncludeRangeInCC.isValid())
+ ReplaceOldInclude(OldHeaderIncludeRangeInCC);
+ else if (Context->Spec.NewHeader == NewFile &&
+ OldHeaderIncludeRangeInHeader.isValid())
+ ReplaceOldInclude(OldHeaderIncludeRangeInHeader);
Context->FileToReplacements[NewFile] = std::move(AllCode);
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
