[clang-tools-extra] r323865 - [clang-move] Clever on handling header file which includes itself.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 31 04:12:29 PST 2018
Author: hokein
Date: Wed Jan 31 04:12:29 2018
New Revision: 323865
URL: http://llvm.org/viewvc/llvm-project?rev=323865&view=rev
Log:
[clang-move] Clever on handling header file which includes itself.
Summary:
Previously, we assume only old.cc includes "old.h", which would
introduce incorrect fixes for the cases where old.h also includes `#include "old.h"`
Although it should not be occurred in real projects, clang-move should handle this.
Old.h:
```
class Foo {};
```
after moving to a new old.h:
```
class Foo {};
```
Reviewers: ioeric
Reviewed By: ioeric
Subscribers: cfe-commits, klimek
Differential Revision: https://reviews.llvm.org/D42639
Modified:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/clang-move/ClangMove.h
clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=323865&r1=323864&r2=323865&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Wed Jan 31 04:12:29 2018
@@ -694,22 +694,28 @@ void ClangMoveTool::addIncludes(llvm::St
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 @@ void ClangMoveTool::moveAll(SourceManage
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);
}
}
Modified: clang-tools-extra/trunk/clang-move/ClangMove.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.h?rev=323865&r1=323864&r2=323865&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.h (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.h Wed Jan 31 04:12:29 2018
@@ -176,7 +176,11 @@ private:
/// 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;
Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=323865&r1=323864&r2=323865&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Wed Jan 31 04:12:29 2018
@@ -293,6 +293,32 @@ TEST(ClangMove, MoveNonExistClass) {
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};",
More information about the cfe-commits
mailing list