[clang-move] Move comments which are associated with the moved class.

Reviewers: ioeric

Subscribers: cfe-commits

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

llvm-svn: 283425
This commit is contained in:
Haojian Wu 2016-10-06 08:59:24 +00:00
parent 253d596d4a
commit 9abbeaad55
4 changed files with 123 additions and 36 deletions

View File

@ -94,6 +94,64 @@ private:
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());
// 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 @@ std::vector<std::string> GetNamespaces(const clang::Decl *D) {
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,
@ -178,8 +216,12 @@ createInsertedReplacements(const std::vector<std::string> &Includes,
// 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 @@ createInsertedReplacements(const std::vector<std::string> &Includes,
++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::addIncludes(llvm::StringRef IncludeHeader,
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]);

View File

@ -27,6 +27,8 @@ class ClangMoveTool : public ast_matchers::MatchFinder::MatchCallback {
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;

View File

@ -70,7 +70,19 @@ cl::opt<bool> Dump("dump_result",
} // 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;

View File

@ -29,8 +29,11 @@ const char TestHeaderName[] = "foo.h";
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 @@ const char TestHeader[] = "namespace a {\n"
"private:\n"
" C1 *c1;\n"
" static int b;\n"
"};\n"
"}; // abc\n"
"\n"
"class Foo2 {\n"
"public:\n"
@ -51,19 +54,29 @@ const char TestCC[] = "#include \"foo.h\"\n"
"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 @@ const char TestCC[] = "#include \"foo.h\"\n"
"} // 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 @@ const char ExpectedTestCC[] = "#include \"foo.h\"\n"
"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 @@ const char ExpectedTestCC[] = "#include \"foo.h\"\n"
"} // 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 @@ const char ExpectedNewHeader[] = "namespace a {\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";
@ -164,8 +195,9 @@ runClangMoveOnCode(const move::ClangMoveTool::MoveDefinitionSpec &Spec) {
Spec, FileToReplacements, InitialDirectory.str(), "LLVM");
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;
@ -183,7 +215,7 @@ TEST(ClangMove, MoveHeaderAndCC) {
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 @@ TEST(ClangMove, MoveCCOnly) {
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]);