Skip to content

Commit

Permalink
[clang-move] The new.cc file should include new_header.h instead of o…
Browse files Browse the repository at this point in the history
…ld_header.h

Summary:
Previously, all #includes (includeing old_header.h) in old.cc will be copied to new.cc,
however, the new.cc should include new_header.h instead of the old_header.h

Before applying the patch, the new.cc looks like:

```
#include "old_header.h"
...
```

The new.cc looks like with this patch:

```
#include "new_header"
...
```

Reviewers: ioeric

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D24828

llvm-svn: 282247
  • Loading branch information
hokein committed Sep 23, 2016
1 parent d2c3c51 commit daf4cb8
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 18 deletions.
31 changes: 18 additions & 13 deletions clang-tools-extra/clang-move/ClangMove.cpp
Expand Up @@ -41,15 +41,8 @@ class FindAllIncludes : public clang::PPCallbacks {
const clang::FileEntry * /*File*/,
StringRef /*SearchPath*/, StringRef /*RelativePath*/,
const clang::Module * /*Imported*/) override {
if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc))) {
if (IsAngled) {
MoveTool->addIncludes("#include <" + FileName.str() + ">\n",
FileEntry->getName());
} else {
MoveTool->addIncludes("#include \"" + FileName.str() + "\"\n",
FileEntry->getName());
}
}
if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))
MoveTool->addIncludes(FileName, IsAngled, FileEntry->getName());
}

private:
Expand Down Expand Up @@ -135,7 +128,7 @@ createInsertedReplacements(const std::vector<std::string> &Includes,

// Add #Includes.
std::string AllIncludesString;
// FIXME: Filter out the old_header.h and add header guard.
// FIXME: Add header guard.
for (const auto &Include : Includes)
AllIncludesString += Include;
clang::tooling::Replacement InsertInclude(FileName, 0, 0, AllIncludesString);
Expand Down Expand Up @@ -205,6 +198,8 @@ ClangMoveTool::ClangMoveTool(
std::map<std::string, tooling::Replacements> &FileToReplacements)
: Spec(MoveSpec), FileToReplacements(FileToReplacements) {
Spec.Name = llvm::StringRef(Spec.Name).ltrim(':');
if (!Spec.NewHeader.empty())
CCIncludes.push_back("#include \"" + Spec.NewHeader + "\"\n");
}

void ClangMoveTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
Expand Down Expand Up @@ -290,12 +285,22 @@ void ClangMoveTool::run(const ast_matchers::MatchFinder::MatchResult &Result) {
}
}

void ClangMoveTool::addIncludes(llvm::StringRef IncludeLine,
void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
llvm::StringRef FileName) {
// FIXME: Add old.h to the new.cc/h when the new target has dependencies on
// old.h/c. For instance, when moved class uses another class defined in
// old.h, the old.h should be added in new.h.
if (!Spec.OldHeader.empty() &&
llvm::StringRef(Spec.OldHeader).endswith(IncludeHeader))
return;

std::string IncludeLine =
IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
: ("#include \"" + IncludeHeader + "\"\n").str();
if (!Spec.OldHeader.empty() && FileName.endswith(Spec.OldHeader))
HeaderIncludes.push_back(IncludeLine.str());
HeaderIncludes.push_back(IncludeLine);
else if (!Spec.OldCC.empty() && FileName.endswith(Spec.OldCC))
CCIncludes.push_back(IncludeLine.str());
CCIncludes.push_back(IncludeLine);
}

void ClangMoveTool::removeClassDefinitionInOldFiles() {
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clang-move/ClangMove.h
Expand Up @@ -55,7 +55,8 @@ class ClangMoveTool : public ast_matchers::MatchFinder::MatchCallback {

// Add #includes from old.h/cc files. The FileName is where the #include
// comes from.
void addIncludes(llvm::StringRef IncludeLine, llvm::StringRef FileName);
void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
llvm::StringRef FileName);

private:
void removeClassDefinitionInOldFiles();
Expand Down
9 changes: 5 additions & 4 deletions clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
Expand Up @@ -119,8 +119,7 @@ const char ExpectedNewHeader[] = "namespace a {\n"
"} // namespace b\n"
"} // namespace a\n";

const char ExpectedNewCC[] = "#include \"foo.h\"\n"
"namespace a {\n"
const char ExpectedNewCC[] = "namespace a {\n"
"namespace b {\n"
"namespace {\n"
"void f1() {}\n"
Expand Down Expand Up @@ -181,11 +180,12 @@ TEST(ClangMove, MoveHeaderAndCC) {
Spec.OldCC = "foo.cc";
Spec.NewHeader = "new_foo.h";
Spec.NewCC = "new_foo.cc";
std::string ExpectedHeader = "#include \"" + Spec.NewHeader + "\"\n";
auto Results = runClangMoveOnCode(Spec);
EXPECT_EQ(ExpectedTestHeader, Results[Spec.OldHeader]);
EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
EXPECT_EQ(ExpectedHeader + ExpectedNewCC, Results[Spec.NewCC]);
}

TEST(ClangMove, MoveHeaderOnly) {
Expand All @@ -204,10 +204,11 @@ TEST(ClangMove, MoveCCOnly) {
Spec.Name = "a::b::Foo";
Spec.OldCC = "foo.cc";
Spec.NewCC = "new_foo.cc";
std::string ExpectedHeader = "#include \"foo.h\"\n";
auto Results = runClangMoveOnCode(Spec);
EXPECT_EQ(2u, Results.size());
EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
EXPECT_EQ(ExpectedHeader + ExpectedNewCC, Results[Spec.NewCC]);
}

TEST(ClangMove, MoveNonExistClass) {
Expand Down

0 comments on commit daf4cb8

Please sign in to comment.