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 @@ -27,6 +27,8 @@ public: // Information about the declaration being moved. struct MovedDecl { + // FIXME: Replace Decl with SourceRange to get rid of calculating range for + // the Decl duplicately. const clang::NamedDecl *Decl = nullptr; clang::SourceManager *SM = nullptr; MovedDecl() = default; 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 @@ -94,6 +94,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 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 Line; + // FIXME: this is a bit hacky to get ReadToEndOfLine work. + Lex.setParsingPreprocessorDirective(true); + Lex.ReadToEndOfLine(&Line); + SourceLocation EndLoc = D->getLocEnd().getLocWithOffset(Line.size()); + // If we already reach EOF, just return the EOF SourceLocation; + // otherwise, move 1 offset ahead to include the trailing newline character + // '\n'. + return SM->getLocForEndOfFile(LocInfo.first) == EndLoc + ? EndLoc + : 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) { @@ -147,26 +205,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 &Includes, const std::vector &Decls, @@ -178,8 +216,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. @@ -213,7 +255,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); @@ -366,10 +407,10 @@ void ClangMoveTool::removeClassDefinitionInOldFiles() { for (const auto &MovedDecl : RemovedDecls) { const auto &SM = *MovedDecl.SM; - auto EndLoc = getLocForEndOfDecl(MovedDecl.Decl, &SM); + auto Range = GetFullRange(&SM, MovedDecl.Decl); clang::tooling::Replacement RemoveReplacement( - 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]); Index: clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp =================================================================== --- clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp +++ clang-tools-extra/trunk/clang-move/tool/ClangMoveMain.cpp @@ -70,7 +70,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 ExtraArgs( argv, argv + argc ); + ExtraArgs.insert(ExtraArgs.begin()+1, "-extra-arg=-fparse-all-comments"); + std::unique_ptr 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-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 @@ -29,8 +29,11 @@ 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" @@ -38,7 +41,7 @@ "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,8 +124,11 @@ "} // 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" @@ -115,22 +136,32 @@ "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"; @@ -164,8 +195,9 @@ Spec, FileToReplacements, InitialDirectory.str(), "LLVM"); tooling::runToolOnCodeWithArgs( - Factory->create(), TestCC, {"-std=c++11"}, TestCCName, "clang-move", - std::make_shared(), FileToSourceText); + Factory->create(), TestCC, {"-std=c++11", "-fparse-all-comments"}, + TestCCName, "clang-move", std::make_shared(), + FileToSourceText); formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite, "llvm"); // The Key is file name, value is the new code after moving the class. std::map Results; @@ -183,7 +215,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]); @@ -207,7 +239,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]);