hokein updated this revision to Diff 77794.
hokein added a comment.
Add comments and update patch descriptions.
https://reviews.llvm.org/D26493
Files:
clang-move/ClangMove.cpp
unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -127,8 +127,10 @@
const char ExpectedNewHeader[] = "#ifndef NEW_FOO_H\n"
"#define NEW_FOO_H\n"
+ "\n"
"namespace a {\n"
"class C1; // test\n"
+ "\n"
"template <typename T> class C2;\n"
"namespace b {\n"
"// This is a Foo class\n"
@@ -144,6 +146,7 @@
"}; // abc\n"
"} // namespace b\n"
"} // namespace a\n"
+ "\n"
"#endif // NEW_FOO_H\n";
const char ExpectedNewCC[] = "namespace a {\n"
@@ -154,17 +157,21 @@
"/// 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"
@@ -312,15 +319,17 @@
TEST(ClangMove, DontMoveAll) {
const char ExpectedHeader[] = "#ifndef NEW_FOO_H\n"
"#define NEW_FOO_H\n"
+ "\n"
"class A {\npublic:\n int f();\n};\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};",
+ "typedef int Int;\nclass A {\npublic:\n int f();\n};\n",
+ "using Int=int;\nclass A {\npublic:\n int f();\n};\n",
+ "class B {};\nclass A {\npublic:\n int f();\n};\n",
+ "void f() {};\nclass A {\npublic:\n int f();\n};\n",
+ "enum Color { RED };\nclass A {\npublic:\n int f();\n};\n",
};
move::ClangMoveTool::MoveDefinitionSpec Spec;
Spec.Names.push_back("A");
@@ -332,7 +341,7 @@
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);
+ std::string ExpectedOldHeader = Header.substr(0, Header.size() - 32);
EXPECT_EQ(ExpectedOldHeader, Results[Spec.OldHeader]);
}
}
Index: clang-move/ClangMove.cpp
===================================================================
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -237,7 +237,7 @@
}
GuardName = StringRef(GuardName).upper();
NewCode += "#ifndef " + GuardName + "\n";
- NewCode += "#define " + GuardName + "\n";
+ NewCode += "#define " + GuardName + "\n\n";
}
// Add #Includes.
@@ -249,40 +249,58 @@
// Add moved class definition and its related declarations. All declarations
// in same namespace are grouped together.
+ //
+ // Record namespaces where the current position is in.
std::vector<std::string> CurrentNamespaces;
for (const auto &MovedDecl : Decls) {
+ // The namespaces of the declaration being moved.
std::vector<std::string> DeclNamespaces = GetNamespaces(MovedDecl.Decl);
auto CurrentIt = CurrentNamespaces.begin();
auto DeclIt = DeclNamespaces.begin();
+ // Skip the common prefix.
while (CurrentIt != CurrentNamespaces.end() &&
DeclIt != DeclNamespaces.end()) {
if (*CurrentIt != *DeclIt)
break;
++CurrentIt;
++DeclIt;
}
+ // Calculate the new namespaces after adding MovedDecl in CurrentNamespace,
+ // which is used for next iteration of this loop.
std::vector<std::string> NextNamespaces(CurrentNamespaces.begin(),
CurrentIt);
NextNamespaces.insert(NextNamespaces.end(), DeclIt, DeclNamespaces.end());
+
+
+ // End with CurrentNamespace.
auto RemainingSize = CurrentNamespaces.end() - CurrentIt;
for (auto It = CurrentNamespaces.rbegin(); RemainingSize > 0;
--RemainingSize, ++It) {
assert(It < CurrentNamespaces.rend());
- NewCode += "} // namespace " + *It + "\n";
+ NewCode += "} // namespace " + *It + "\n\n";
}
+
+ // If the moved declaration is not in CurrentNamespace, add extra namespace
+ // definitions.
+ bool IsInNewNamespace = false;
while (DeclIt != DeclNamespaces.end()) {
NewCode += "namespace " + *DeclIt + " {\n";
+ IsInNewNamespace = true;
++DeclIt;
}
+ // If the moved declaration is in same namespace CurrentNamespace, add
+ // a preceeding `\n' before the moved declaration.
+ if (!IsInNewNamespace)
+ NewCode += "\n";
NewCode += getDeclarationSourceText(MovedDecl.Decl, MovedDecl.SM);
CurrentNamespaces = std::move(NextNamespaces);
}
std::reverse(CurrentNamespaces.begin(), CurrentNamespaces.end());
for (const auto &NS : CurrentNamespaces)
NewCode += "} // namespace " + NS + "\n";
if (IsHeader)
- NewCode += "#endif // " + GuardName + "\n";
+ NewCode += "\n#endif // " + GuardName + "\n";
return clang::tooling::Replacements(
clang::tooling::Replacement(FileName, 0, 0, NewCode));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
