This revision was automatically updated to reflect the committed changes.
Closed by commit rL286281: [clang-move] Move all code from old.h/cc directly
when moving all class… (authored by hokein).
Changed prior to commit:
https://reviews.llvm.org/D26236?vs=77223&id=77233#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26236
Files:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/clang-move/ClangMove.h
clang-tools-extra/trunk/test/clang-move/Inputs/test.h
clang-tools-extra/trunk/test/clang-move/move-class.cpp
clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
Index: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
@@ -173,24 +173,25 @@
"} // namespace a\n";
std::map<std::string, std::string>
-runClangMoveOnCode(const move::ClangMoveTool::MoveDefinitionSpec &Spec) {
+runClangMoveOnCode(const move::ClangMoveTool::MoveDefinitionSpec &Spec,
+ const char *const Header = TestHeader,
+ const char *const CC = TestCC) {
clang::RewriterTestContext Context;
std::map<llvm::StringRef, clang::FileID> FileToFileID;
std::vector<std::pair<std::string, std::string>> FileToSourceText = {
- {TestHeaderName, TestHeader}, {TestCCName, TestCC}};
+ {TestHeaderName, Header}, {TestCCName, CC}};
auto CreateFiles = [&FileToSourceText, &Context, &FileToFileID](
llvm::StringRef Name, llvm::StringRef Code) {
if (!Name.empty()) {
- FileToSourceText.emplace_back(Name, Code);
FileToFileID[Name] = Context.createInMemoryFile(Name, Code);
}
};
CreateFiles(Spec.NewCC, "");
CreateFiles(Spec.NewHeader, "");
- CreateFiles(Spec.OldHeader, TestHeader);
- CreateFiles(Spec.OldCC, TestCC);
+ CreateFiles(Spec.OldHeader, Header);
+ CreateFiles(Spec.OldCC, CC);
std::map<std::string, tooling::Replacements> FileToReplacements;
llvm::SmallString<128> InitialDirectory;
@@ -201,7 +202,7 @@
Spec, FileToReplacements, InitialDirectory.str(), "LLVM");
tooling::runToolOnCodeWithArgs(
- Factory->create(), TestCC, {"-std=c++11", "-fparse-all-comments"},
+ Factory->create(), CC, {"-std=c++11", "-fparse-all-comments"},
TestCCName, "clang-move", std::make_shared<PCHContainerOperations>(),
FileToSourceText);
formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm");
@@ -263,6 +264,79 @@
EXPECT_EQ(0u, Results.size());
}
+TEST(ClangMove, MoveAll) {
+ std::vector<std::string> TestHeaders = {
+ "class A {\npublic:\n int f();\n};",
+ // forward declaration.
+ "class B;\nclass A {\npublic:\n int f();\n};",
+ // template forward declaration.
+ "template <typename T> class B;\nclass A {\npublic:\n int f();\n};",
+ "namespace a {}\nclass A {\npublic:\n int f();\n};",
+ "namespace a {}\nusing namespace a;\nclass A {\npublic:\n int f();\n};",
+ };
+ const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }";
+ 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";
+ for (const auto& Header : TestHeaders) {
+ auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code);
+ EXPECT_EQ(Header, Results[Spec.NewHeader]);
+ EXPECT_EQ("", Results[Spec.OldHeader]);
+ EXPECT_EQ("", Results[Spec.OldCC]);
+ }
+}
+
+TEST(ClangMove, MoveAllMultipleClasses) {
+ move::ClangMoveTool::MoveDefinitionSpec Spec;
+ std::vector<std::string> TestHeaders = {
+ "class C;\nclass A {\npublic:\n int f();\n};\nclass B {};",
+ "class C;\nclass B;\nclass A {\npublic:\n int f();\n};\nclass B {};",
+ };
+ const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }";
+ Spec.Names = {std::string("A"), std::string("B")};
+ Spec.OldHeader = "foo.h";
+ Spec.OldCC = "foo.cc";
+ Spec.NewHeader = "new_foo.h";
+ Spec.NewCC = "new_foo.cc";
+ for (const auto& Header : TestHeaders) {
+ auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code);
+ EXPECT_EQ(Header, Results[Spec.NewHeader]);
+ EXPECT_EQ("", Results[Spec.OldHeader]);
+ EXPECT_EQ("", Results[Spec.OldCC]);
+ }
+}
+
+TEST(ClangMove, DontMoveAll) {
+ const char ExpectedHeader[] = "#ifndef NEW_FOO_H\n"
+ "#define NEW_FOO_H\n"
+ "class A {\npublic:\n int f();\n};\n"
+ "#endif // NEW_FOO_H\n";
+ const char Code[] = "#include \"foo.h\"\nint A::f() { return 0; }";
+ std::vector<std::string> TestHeaders = {
+ "typedef int Int;\nclass A {\npublic:\n int f();\n};",
+ "using Int=int;\nclass A {\npublic:\n int f();\n};",
+ "class B {};\nclass A {\npublic:\n int f();\n};",
+ "void f() {};\nclass A {\npublic:\n int f();\n};",
+ "enum Color { RED };\nclass A {\npublic:\n int f();\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";
+ for (const auto& Header : TestHeaders) {
+ auto Results = runClangMoveOnCode(Spec, Header.c_str(), Code);
+ EXPECT_EQ(ExpectedHeader, Results[Spec.NewHeader]);
+ // The expected old header should not contain class A definition.
+ std::string ExpectedOldHeader = Header.substr(0, Header.size() - 31);
+ EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]);
+ }
+}
+
} // namespace
} // namespce move
} // namespace clang
Index: clang-tools-extra/trunk/clang-move/ClangMove.cpp
===================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp
@@ -122,13 +122,13 @@
void InclusionDirective(clang::SourceLocation HashLoc,
const clang::Token & /*IncludeTok*/,
StringRef FileName, bool IsAngled,
- clang::CharSourceRange /*FilenameRange*/,
+ clang::CharSourceRange FilenameRange,
const clang::FileEntry * /*File*/,
StringRef SearchPath, StringRef /*RelativePath*/,
const clang::Module * /*Imported*/) override {
if (const auto *FileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)))
MoveTool->addIncludes(FileName, IsAngled, SearchPath,
- FileEntry->getName(), SM);
+ FileEntry->getName(), FilenameRange, SM);
}
private:
@@ -321,28 +321,53 @@
return;
}
- auto InOldHeader = isExpansionInFile(
- MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader));
- auto InOldCC =
- isExpansionInFile(MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC));
+ auto InOldHeader = isExpansionInFile(makeAbsolutePath(Spec.OldHeader));
+ auto InOldCC = isExpansionInFile(makeAbsolutePath(Spec.OldCC));
auto InOldFiles = anyOf(InOldHeader, InOldCC);
auto InMovedClass =
hasOutermostEnclosingClass(cxxRecordDecl(*InMovedClassNames));
+ auto ForwardDecls =
+ cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition())));
+
+ //============================================================================
+ // Matchers for old header
+ //============================================================================
+ // Match all top-level named declarations (e.g. function, variable, enum) in
+ // old header, exclude forward class declarations and namespace declarations.
+ //
+ // The old header which contains only one declaration being moved and forward
+ // declarations is considered to be moved totally.
+ auto AllDeclsInHeader = namedDecl(
+ unless(ForwardDecls), unless(namespaceDecl()),
+ unless(usingDirectiveDecl()), // using namespace decl.
+ unless(classTemplateDecl(has(ForwardDecls))), // template forward decl.
+ InOldHeader,
+ hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl()))));
+ Finder->addMatcher(AllDeclsInHeader.bind("decls_in_header"), this);
+ // Match forward declarations in old header.
+ Finder->addMatcher(namedDecl(ForwardDecls, InOldHeader).bind("fwd_decl"),
+ this);
+
+ //============================================================================
+ // Matchers for old files, including old.h/old.cc
+ //============================================================================
// Match moved class declarations.
auto MovedClass = cxxRecordDecl(
InOldFiles, *InMovedClassNames, isDefinition(),
hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())));
Finder->addMatcher(MovedClass.bind("moved_class"), this);
-
// Match moved class methods (static methods included) which are defined
// outside moved class declaration.
Finder->addMatcher(
cxxMethodDecl(InOldFiles, ofOutermostEnclosingClass(*InMovedClassNames),
isDefinition())
.bind("class_method"),
this);
+ //============================================================================
+ // Matchers for old cc
+ //============================================================================
// Match static member variable definition of the moved class.
Finder->addMatcher(
varDecl(InMovedClass, InOldCC, isDefinition(), isStaticDataMember())
@@ -374,16 +399,13 @@
varDecl(IsOldCCStaticDefinition)))
.bind("static_decls"),
this);
-
- // Match forward declarations in old header.
- Finder->addMatcher(
- cxxRecordDecl(unless(anyOf(isImplicit(), isDefinition())), InOldHeader)
- .bind("fwd_decl"),
- this);
}
void ClangMoveTool::run(const ast_matchers::MatchFinder::MatchResult &Result) {
- if (const auto *CMD =
+ if (const auto *D =
+ Result.Nodes.getNodeAs<clang::NamedDecl>("decls_in_header")) {
+ UnremovedDeclsInOldHeader.insert(D);
+ } else if (const auto *CMD =
Result.Nodes.getNodeAs<clang::CXXMethodDecl>("class_method")) {
// Skip inline class methods. isInline() ast matcher doesn't ignore this
// case.
@@ -399,6 +421,7 @@
Result.Nodes.getNodeAs<clang::CXXRecordDecl>("moved_class")) {
MovedDecls.emplace_back(class_decl, &Result.Context->getSourceManager());
RemovedDecls.push_back(MovedDecls.back());
+ UnremovedDeclsInOldHeader.erase(class_decl);
} else if (const auto *FWD =
Result.Nodes.getNodeAs<clang::CXXRecordDecl>("fwd_decl")) {
// Skip all forwad declarations which appear after moved class declaration.
@@ -421,31 +444,36 @@
}
}
+std::string ClangMoveTool::makeAbsolutePath(StringRef Path) {
+ return MakeAbsolutePath(OriginalRunningDirectory, Path);
+}
+
void ClangMoveTool::addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
llvm::StringRef SearchPath,
llvm::StringRef FileName,
+ clang::CharSourceRange IncludeFilenameRange,
const SourceManager &SM) {
SmallVector<char, 128> HeaderWithSearchPath;
llvm::sys::path::append(HeaderWithSearchPath, SearchPath, IncludeHeader);
- std::string AbsoluteOldHeader =
- MakeAbsolutePath(OriginalRunningDirectory, Spec.OldHeader);
+ std::string AbsoluteOldHeader = makeAbsolutePath(Spec.OldHeader);
// 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 (AbsoluteOldHeader ==
MakeAbsolutePath(SM, llvm::StringRef(HeaderWithSearchPath.data(),
- HeaderWithSearchPath.size())))
+ HeaderWithSearchPath.size()))) {
+ OldHeaderIncludeRange = IncludeFilenameRange;
return;
+ }
std::string IncludeLine =
IsAngled ? ("#include <" + IncludeHeader + ">\n").str()
: ("#include \"" + IncludeHeader + "\"\n").str();
std::string AbsoluteCurrentFile = MakeAbsolutePath(SM, FileName);
if (AbsoluteOldHeader == AbsoluteCurrentFile) {
HeaderIncludes.push_back(IncludeLine);
- } else if (MakeAbsolutePath(OriginalRunningDirectory, Spec.OldCC) ==
- AbsoluteCurrentFile) {
+ } else if (makeAbsolutePath(Spec.OldCC) == AbsoluteCurrentFile) {
CCIncludes.push_back(IncludeLine);
}
}
@@ -499,9 +527,46 @@
createInsertedReplacements(CCIncludes, NewCCDecls, Spec.NewCC);
}
+// Move all contents from OldFile to NewFile.
+void ClangMoveTool::moveAll(SourceManager &SM, StringRef OldFile,
+ StringRef NewFile) {
+ const FileEntry *FE = SM.getFileManager().getFile(makeAbsolutePath(OldFile));
+ if (!FE) {
+ llvm::errs() << "Failed to get file: " << OldFile << "\n";
+ return;
+ }
+ FileID ID = SM.getOrCreateFileID(FE, SrcMgr::C_User);
+ auto Begin = SM.getLocForStartOfFile(ID);
+ auto End = SM.getLocForEndOfFile(ID);
+ clang::tooling::Replacement RemoveAll (
+ SM, clang::CharSourceRange::getCharRange(Begin, End), "");
+ std::string FilePath = RemoveAll.getFilePath().str();
+ FileToReplacements[FilePath] = clang::tooling::Replacements(RemoveAll);
+
+ StringRef Code = SM.getBufferData(ID);
+ 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 (Spec.NewCC == NewFile && OldHeaderIncludeRange.isValid()) {
+ AllCode = AllCode.merge(
+ clang::tooling::Replacements(clang::tooling::Replacement(
+ SM, OldHeaderIncludeRange, '"' + Spec.NewHeader + '"')));
+ }
+ FileToReplacements[NewFile] = std::move(AllCode);
+ }
+}
+
void ClangMoveTool::onEndOfTranslationUnit() {
if (RemovedDecls.empty())
return;
+ if (UnremovedDeclsInOldHeader.empty() && !Spec.OldHeader.empty()) {
+ auto &SM = *RemovedDecls[0].SM;
+ moveAll(SM, Spec.OldHeader, Spec.NewHeader);
+ moveAll(SM, Spec.OldCC, Spec.NewCC);
+ return;
+ }
removeClassDefinitionInOldFiles();
moveClassDefinitionToNewFiles();
}
Index: clang-tools-extra/trunk/clang-move/ClangMove.h
===================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.h
+++ clang-tools-extra/trunk/clang-move/ClangMove.h
@@ -14,6 +14,7 @@
#include "clang/Frontend/FrontendAction.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include <map>
#include <string>
#include <vector>
@@ -23,6 +24,9 @@
// FIXME: Make it support more types, e.g. function definitions.
// Currently only support moving class definition.
+//
+// When moving all class declarations in old header, all code in old.h/cc will
+// be moved.
class ClangMoveTool : public ast_matchers::MatchFinder::MatchCallback {
public:
// Information about the declaration being moved.
@@ -68,14 +72,22 @@
/// \param SearchPath The search path which was used to find the IncludeHeader
/// in the file system. It can be a relative path or an absolute path.
/// \param FileName The name of file where the IncludeHeader comes from.
+ /// \param IncludeRange The source range for the written file name in #include
+ /// (i.e. "old.h" for #include "old.h") in old.cc.
/// \param SM The SourceManager.
void addIncludes(llvm::StringRef IncludeHeader, bool IsAngled,
llvm::StringRef SearchPath, llvm::StringRef FileName,
+ clang::CharSourceRange IncludeFilenameRange,
const SourceManager &SM);
private:
+ // Make the Path absolute using the OrignalRunningDirectory if the Path is not
+ // an absolute path. An empty Path will result in an empty string.
+ std::string makeAbsolutePath(StringRef Path);
+
void removeClassDefinitionInOldFiles();
void moveClassDefinitionToNewFiles();
+ void moveAll(SourceManager& SM, StringRef OldFile, StringRef NewFile);
MoveDefinitionSpec Spec;
// The Key is file path, value is the replacements being applied to the file.
@@ -97,6 +109,12 @@
std::string OriginalRunningDirectory;
// The name of a predefined code style.
std::string FallbackStyle;
+ // The unmoved named declarations in old header.
+ llvm::SmallPtrSet<const NamedDecl*, 8> UnremovedDeclsInOldHeader;
+ /// 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;
};
class ClangMoveAction : public clang::ASTFrontendAction {
Index: clang-tools-extra/trunk/test/clang-move/Inputs/test.h
===================================================================
--- clang-tools-extra/trunk/test/clang-move/Inputs/test.h
+++ clang-tools-extra/trunk/test/clang-move/Inputs/test.h
@@ -1,7 +1,10 @@
+#ifndef TEST_H // comment 1
+#define TEST_H
namespace a {
class Foo {
public:
int f();
int f2(int a, int b);
};
} // namespace a
+#endif // TEST_H
Index: clang-tools-extra/trunk/test/clang-move/move-class.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-move/move-class.cpp
+++ clang-tools-extra/trunk/test/clang-move/move-class.cpp
@@ -9,36 +9,35 @@
// RUN: clang-move -names="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=../src/test.cpp -old_header=../include/test.h %T/clang-move/src/test.cpp
// RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
// RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s
-// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
-// RUN: FileCheck -input-file=%T/clang-move/include/test.h %s -implicit-check-not='{{namespace.*}}'
+// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s
+// RUN: FileCheck -input-file=%T/clang-move/include/test.h -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s
//
// RUN: cp %S/Inputs/test.h %T/clang-move/include
// RUN: cp %S/Inputs/test.cpp %T/clang-move/src
// RUN: cd %T/clang-move/build
// RUN: clang-move -names="a::Foo" -new_cc=%T/clang-move/new_test.cpp -new_header=%T/clang-move/new_test.h -old_cc=%T/clang-move/src/test.cpp -old_header=%T/clang-move/include/test.h %T/clang-move/src/test.cpp
// RUN: FileCheck -input-file=%T/clang-move/new_test.cpp -check-prefix=CHECK-NEW-TEST-CPP %s
// RUN: FileCheck -input-file=%T/clang-move/new_test.h -check-prefix=CHECK-NEW-TEST-H %s
-// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-CPP %s
-// RUN: FileCheck -input-file=%T/clang-move/include/test.h %s -implicit-check-not='{{namespace.*}}'
+// RUN: FileCheck -input-file=%T/clang-move/src/test.cpp -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s
+// RUN: FileCheck -input-file=%T/clang-move/include/test.h -check-prefix=CHECK-OLD-TEST-EMPTY -allow-empty %s
//
//
-// CHECK-NEW-TEST-H: #ifndef {{.*}}CLANG_MOVE_NEW_TEST_H
-// CHECK-NEW-TEST-H: #define {{.*}}CLANG_MOVE_NEW_TEST_H
+// CHECK-NEW-TEST-H: #ifndef TEST_H // comment 1
+// CHECK-NEW-TEST-H: #define TEST_H
// CHECK-NEW-TEST-H: namespace a {
// CHECK-NEW-TEST-H: class Foo {
// CHECK-NEW-TEST-H: public:
// CHECK-NEW-TEST-H: int f();
// CHECK-NEW-TEST-H: int f2(int a, int b);
// CHECK-NEW-TEST-H: };
// CHECK-NEW-TEST-H: } // namespace a
-// CHECK-NEW-TEST-H: #endif // {{.*}}CLANG_MOVE_NEW_TEST_H
+// CHECK-NEW-TEST-H: #endif // TEST_H
//
// CHECK-NEW-TEST-CPP: #include "{{.*}}new_test.h"
// CHECK-NEW-TEST-CPP: #include "test2.h"
// CHECK-NEW-TEST-CPP: namespace a {
// CHECK-NEW-TEST-CPP: int Foo::f() { return 0; }
// CHECK-NEW-TEST-CPP: int Foo::f2(int a, int b) { return a + b; }
// CHECK-NEW-TEST-CPP: } // namespace a
//
-// CHECK-OLD-TEST-CPP: #include "test.h"
-// CHECK-OLD-TEST-CPP: #include "test2.h"
+// CHECK-OLD-TEST-EMPTY: {{^}}{{$}}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
