hokein created this revision.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.
https://reviews.llvm.org/D25227
Files:
clang-move/ClangMove.cpp
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
@@ -29,16 +29,19 @@
const char TestCCName[] = "foo.cc";
const char TestHeader[] = "namespace a {\n"
- "class C1;\n"
+ "class C1; // test\n"
"namespace b {\n"
+ "// This is a Foo class\n"
+ "// which is used in\n"
+ "// test.\n"
"class Foo {\n"
"public:\n"
" void f();\n"
"\n"
"private:\n"
" C1 *c1;\n"
" static int b;\n"
- "};\n"
+ "}; // abc\n"
"\n"
"class Foo2 {\n"
"public:\n"
@@ -51,19 +54,29 @@
"namespace a {\n"
"namespace b {\n"
"namespace {\n"
+ "// comment1.\n"
"void f1() {}\n"
+ "/// comment2.\n"
"int kConstInt1 = 0;\n"
"} // namespace\n"
"\n"
+ "/* comment 3*/\n"
"static int kConstInt2 = 1;\n"
"\n"
+ "/** comment4\n"
+ "*/\n"
"static int help() {\n"
" int a = 0;\n"
" return a;\n"
"}\n"
"\n"
+ "// comment5\n"
+ "// comment5\n"
"void Foo::f() { f1(); }\n"
"\n"
+ "/////////////\n"
+ "// comment //\n"
+ "/////////////\n"
"int Foo::b = 2;\n"
"int Foo2::f() {\n"
" f1();\n"
@@ -73,7 +86,7 @@
"} // namespace a\n";
const char ExpectedTestHeader[] = "namespace a {\n"
- "class C1;\n"
+ "class C1; // test\n"
"namespace b {\n"
"\n"
"class Foo2 {\n"
@@ -87,12 +100,17 @@
"namespace a {\n"
"namespace b {\n"
"namespace {\n"
+ "// comment1.\n"
"void f1() {}\n"
+ "/// comment2.\n"
"int kConstInt1 = 0;\n"
"} // namespace\n"
"\n"
+ "/* comment 3*/\n"
"static int kConstInt2 = 1;\n"
"\n"
+ "/** comment4\n"
+ "*/\n"
"static int help() {\n"
" int a = 0;\n"
" return a;\n"
@@ -106,31 +124,44 @@
"} // namespace a\n";
const char ExpectedNewHeader[] = "namespace a {\n"
- "class C1;\n"
+ "class C1; // test\n"
"namespace b {\n"
+ "// This is a Foo class\n"
+ "// which is used in\n"
+ "// test.\n"
"class Foo {\n"
"public:\n"
" void f();\n"
"\n"
"private:\n"
" C1 *c1;\n"
" static int b;\n"
- "};\n"
+ "}; // abc\n"
"} // namespace b\n"
"} // namespace a\n";
const char ExpectedNewCC[] = "namespace a {\n"
"namespace b {\n"
"namespace {\n"
+ "// comment1.\n"
"void f1() {}\n"
+ "/// comment2.\n"
"int kConstInt1 = 0;\n"
"} // namespace\n"
+ "/* comment 3*/\n"
"static int kConstInt2 = 1;\n"
+ "/** comment4\n"
+ "*/\n"
"static int help() {\n"
" int a = 0;\n"
" return a;\n"
"}\n"
+ "// comment5\n"
+ "// comment5\n"
"void Foo::f() { f1(); }\n"
+ "/////////////\n"
+ "// comment //\n"
+ "/////////////\n"
"int Foo::b = 2;\n"
"} // namespace b\n"
"} // namespace a\n";
@@ -161,8 +192,9 @@
Spec, FileToReplacements);
tooling::runToolOnCodeWithArgs(
- Factory->create(), TestCC, {"-std=c++11"}, TestCCName, "clang-move",
- std::make_shared<PCHContainerOperations>(), FileToSourceText);
+ Factory->create(), TestCC, {"-std=c++11", "-fparse-all-comments"},
+ TestCCName, "clang-move", std::make_shared<PCHContainerOperations>(),
+ FileToSourceText);
formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm");
// The Key is file name, value is the new code after moving the class.
std::map<std::string, std::string> Results;
@@ -180,7 +212,7 @@
Spec.OldCC = "foo.cc";
Spec.NewHeader = "new_foo.h";
Spec.NewCC = "new_foo.cc";
- std::string ExpectedHeader = "#include \"" + Spec.NewHeader + "\"\n";
+ std::string ExpectedHeader = "#include \"" + Spec.NewHeader + "\"\n\n";
auto Results = runClangMoveOnCode(Spec);
EXPECT_EQ(ExpectedTestHeader, Results[Spec.OldHeader]);
EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
@@ -204,7 +236,7 @@
Spec.Name = "a::b::Foo";
Spec.OldCC = "foo.cc";
Spec.NewCC = "new_foo.cc";
- std::string ExpectedHeader = "#include \"foo.h\"\n";
+ std::string ExpectedHeader = "#include \"foo.h\"\n\n";
auto Results = runClangMoveOnCode(Spec);
EXPECT_EQ(2u, Results.size());
EXPECT_EQ(ExpectedTestCC, Results[Spec.OldCC]);
Index: clang-move/tool/ClangMoveMain.cpp
===================================================================
--- clang-move/tool/ClangMoveMain.cpp
+++ clang-move/tool/ClangMoveMain.cpp
@@ -62,7 +62,19 @@
} // namespace
int main(int argc, const char **argv) {
- tooling::CommonOptionsParser OptionsParser(argc, argv, ClangMoveCategory);
+ // Add "-fparse-all-comments" compile option to make clang parse all comments,
+ // otherwise, ordinary comments like "//" and "/*" won't get parsed (This is
+ // a bit of hacky).
+ std::vector<std::string> ExtraArgs( argv, argv + argc );
+ ExtraArgs.insert(ExtraArgs.begin()+1, "-extra-arg=-fparse-all-comments");
+ std::unique_ptr<const char *[]> RawExtraArgs(
+ new const char *[ExtraArgs.size()]);
+ for (size_t i = 0; i < ExtraArgs.size(); ++i)
+ RawExtraArgs[i] = ExtraArgs[i].c_str();
+ int Argc = argc + 1;
+ tooling::CommonOptionsParser OptionsParser(Argc, RawExtraArgs.get(),
+ ClangMoveCategory);
+
tooling::RefactoringTool Tool(OptionsParser.getCompilations(),
OptionsParser.getSourcePathList());
move::ClangMoveTool::MoveDefinitionSpec Spec;
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -44,6 +44,64 @@
ClangMoveTool *const MoveTool;
};
+// Expand to get the end location of the line where the EndLoc of the given
+// Decl.
+SourceLocation
+getLocForEndOfDecl(const clang::Decl *D, const SourceManager *SM,
+ const LangOptions &LangOpts = clang::LangOptions()) {
+ std::pair<FileID, unsigned> LocInfo = SM->getDecomposedLoc(D->getLocEnd());
+ // Try to load the file buffer.
+ bool InvalidTemp = false;
+ llvm::StringRef File = SM->getBufferData(LocInfo.first, &InvalidTemp);
+ if (InvalidTemp)
+ return SourceLocation();
+
+ const char *TokBegin = File.data() + LocInfo.second;
+ // Lex from the start of the given location.
+ Lexer Lex(SM->getLocForStartOfFile(LocInfo.first), LangOpts, File.begin(),
+ TokBegin, File.end());
+
+ llvm::SmallVector<char, 16> Line;
+ // FIXME: this is a bit hacky to get ReadToEndOfLine work.
+ Lex.setParsingPreprocessorDirective(true);
+ Lex.ReadToEndOfLine(&Line);
+ SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size());
+ // Already reach EOF.
+ if (SM->getLocForEndOfFile(LocInfo.first)
+ == EndLoc)
+ return EndLoc;
+ // Include the trailing newline character "\n".
+ return EndLoc.getLocWithOffset(1);
+}
+
+// Get full range of a Decl including the comments associated with it.
+clang::CharSourceRange
+GetFullRange(const clang::SourceManager *SM, const clang::Decl *D,
+ const clang::LangOptions &options = clang::LangOptions()) {
+ clang::SourceRange Full = D->getSourceRange();
+ Full.setEnd(getLocForEndOfDecl(D, SM));
+ // Expand to comments that are associated with the Decl.
+ if (const auto* Comment =
+ D->getASTContext().getRawCommentForDeclNoCache(D)) {
+ if (SM->isBeforeInTranslationUnit(Full.getEnd(), Comment->getLocEnd()))
+ Full.setEnd(Comment->getLocEnd());
+ // FIXME: Don't delete a preceding comment, if there are no other entities
+ // it could refer to.
+ if (SM->isBeforeInTranslationUnit(Comment->getLocStart(),
+ Full.getBegin()))
+ Full.setBegin(Comment->getLocStart());
+ }
+
+ return clang::CharSourceRange::getCharRange(Full);
+}
+
+std::string getDeclarationSourceText(const clang::Decl *D,
+ const clang::SourceManager *SM) {
+ llvm::StringRef SourceText = clang::Lexer::getSourceText(
+ GetFullRange(SM, D), *SM, clang::LangOptions());
+ return SourceText.str();
+}
+
clang::tooling::Replacement
getReplacementInChangedCode(const clang::tooling::Replacements &Replacements,
const clang::tooling::Replacement &Replacement) {
@@ -94,26 +152,6 @@
return Namespaces;
}
-SourceLocation getLocForEndOfDecl(const clang::Decl *D,
- const clang::SourceManager *SM) {
- auto End = D->getLocEnd();
- clang::SourceLocation AfterSemi = clang::Lexer::findLocationAfterToken(
- End, clang::tok::semi, *SM, clang::LangOptions(),
- /*SkipTrailingWhitespaceAndNewLine=*/true);
- if (AfterSemi.isValid())
- End = AfterSemi.getLocWithOffset(-1);
- return End;
-}
-
-std::string getDeclarationSourceText(const clang::Decl *D,
- const clang::SourceManager *SM) {
- auto EndLoc = getLocForEndOfDecl(D, SM);
- llvm::StringRef SourceText = clang::Lexer::getSourceText(
- clang::CharSourceRange::getTokenRange(D->getLocStart(), EndLoc), *SM,
- clang::LangOptions());
- return SourceText.str() + "\n";
-}
-
clang::tooling::Replacements
createInsertedReplacements(const std::vector<std::string> &Includes,
const std::vector<ClangMoveTool::MovedDecl> &Decls,
@@ -125,8 +163,12 @@
// FIXME: Add header guard.
for (const auto &Include : Includes)
AllIncludesString += Include;
- clang::tooling::Replacement InsertInclude(FileName, 0, 0, AllIncludesString);
- addOrMergeReplacement(InsertInclude, &InsertedReplacements);
+
+ if (!AllIncludesString.empty()) {
+ clang::tooling::Replacement InsertInclude(FileName, 0, 0,
+ AllIncludesString + "\n");
+ addOrMergeReplacement(InsertInclude, &InsertedReplacements);
+ }
// Add moved class definition and its related declarations. All declarations
// in same namespace are grouped together.
@@ -160,7 +202,6 @@
++DeclIt;
}
- // FIXME: consider moving comments of the moved declaration.
clang::tooling::Replacement InsertedReplacement(
FileName, 0, 0, getDeclarationSourceText(MovedDecl.Decl, MovedDecl.SM));
addOrMergeReplacement(InsertedReplacement, &InsertedReplacements);
@@ -299,10 +340,10 @@
void ClangMoveTool::removeClassDefinitionInOldFiles() {
for (const auto &MovedDecl : RemovedDecls) {
- auto EndLoc = getLocForEndOfDecl(MovedDecl.Decl, MovedDecl.SM);
+ auto Range = GetFullRange(MovedDecl.SM, MovedDecl.Decl);
clang::tooling::Replacement RemoveReplacement(
- *MovedDecl.SM, clang::CharSourceRange::getTokenRange(
- MovedDecl.Decl->getLocStart(), EndLoc),
+ *MovedDecl.SM, clang::CharSourceRange::getCharRange(
+ Range.getBegin(), Range.getEnd()),
"");
std::string FilePath = RemoveReplacement.getFilePath().str();
addOrMergeReplacement(RemoveReplacement, &FileToReplacements[FilePath]);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
