[clang-tools-extra] r286943 - [clang-move] Make the output code look more pretty.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 15 01:07:00 PST 2016
Author: hokein
Date: Tue Nov 15 03:06:59 2016
New Revision: 286943
URL: http://llvm.org/viewvc/llvm-project?rev=286943&view=rev
Log:
[clang-move] Make the output code look more pretty.
Summary:
Add decent blank lines between declarations:
* Add extra blank line after #define or #includes.
* Add extra blank line between declarations.
* Add extra blank line in front of #endif.
Previously, the new generated code is quite tight:
```
#ifndef FOO_H
#define FOO_H
namespace a {
class A { public: int f(); };
int A::f() { return 0; }
} // namespace a
#endif // FOO_H
```
After this patch, the code looks like:
```
#ifndef FOO_H
#define FOO_H
namespace a {
class A { public: int f(); };
int A::f() { return 0; }
} // namespace a
#endif // FOO_H
```
Reviewers: ioeric
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D26493
Modified:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=286943&r1=286942&r2=286943&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Tue Nov 15 03:06:59 2016
@@ -294,7 +294,7 @@ createInsertedReplacements(const std::ve
}
GuardName = StringRef(GuardName).upper();
NewCode += "#ifndef " + GuardName + "\n";
- NewCode += "#define " + GuardName + "\n";
+ NewCode += "#define " + GuardName + "\n\n";
}
// Add #Includes.
@@ -306,11 +306,15 @@ createInsertedReplacements(const std::ve
// 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)
@@ -318,19 +322,38 @@ createInsertedReplacements(const std::ve
++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.
+ bool HasEndCurrentNamespace = false;
auto RemainingSize = CurrentNamespaces.end() - CurrentIt;
for (auto It = CurrentNamespaces.rbegin(); RemainingSize > 0;
--RemainingSize, ++It) {
assert(It < CurrentNamespaces.rend());
NewCode += "} // namespace " + *It + "\n";
+ HasEndCurrentNamespace = true;
}
+ // Add trailing '\n' after the nested namespace definition.
+ if (HasEndCurrentNamespace)
+ NewCode += "\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);
}
@@ -339,7 +362,7 @@ createInsertedReplacements(const std::ve
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));
}
Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=286943&r1=286942&r2=286943&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Tue Nov 15 03:06:59 2016
@@ -127,8 +127,10 @@ const char ExpectedTestCC[] = "#include
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 @@ const char ExpectedNewHeader[] = "#ifnde
"}; // abc\n"
"} // namespace b\n"
"} // namespace a\n"
+ "\n"
"#endif // NEW_FOO_H\n";
const char ExpectedNewCC[] = "namespace a {\n"
@@ -154,17 +157,21 @@ const char ExpectedNewCC[] = "namespace
"/// 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, MoveAllMultipleClasses)
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 @@ TEST(ClangMove, DontMoveAll) {
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]);
}
}
@@ -355,6 +364,61 @@ TEST(ClangMove, MacroInFunction) {
EXPECT_EQ(ExpectedNewCode, Results[Spec.NewCC]);
}
+TEST(ClangMove, WellFormattedCode) {
+ const std::string CommonHeader =
+ "namespace a {\n"
+ "namespace b {\n"
+ "namespace c {\n"
+ "class C;\n"
+ "\n"
+ "class A {\npublic:\n void f();\n void f2();\n};\n"
+ "} // namespace c\n"
+ "} // namespace b\n"
+ "\n"
+ "namespace d {\n"
+ "namespace e {\n"
+ "class B {\npublic:\n void f();\n};\n"
+ "} // namespace e\n"
+ "} // namespace d\n"
+ "} // namespace a\n";
+ const std::string CommonCode = "\n"
+ "namespace a {\n"
+ "namespace b {\n"
+ "namespace c {\n"
+ "void A::f() {}\n"
+ "\n"
+ "void A::f2() {}\n"
+ "} // namespace c\n"
+ "} // namespace b\n"
+ "\n"
+ "namespace d {\n"
+ "namespace e {\n"
+ "void B::f() {}\n"
+ "} // namespace e\n"
+ "} // namespace d\n"
+ "} // namespace a\n";
+ // Add dummy class to prevent behavior of moving all declarations from header.
+ const std::string TestHeader = CommonHeader + "class D {};\n";
+ const std::string TestCode = "#include \"foo.h\"\n" + CommonCode;
+ const std::string ExpectedNewHeader = "#ifndef NEW_FOO_H\n"
+ "#define NEW_FOO_H\n"
+ "\n" +
+ CommonHeader +
+ "\n"
+ "#endif // NEW_FOO_H\n";
+ const std::string ExpectedNewCC = "#include \"new_foo.h\"\n" + CommonCode;
+ move::ClangMoveTool::MoveDefinitionSpec Spec;
+ Spec.Names.push_back("a::b::c::A");
+ Spec.Names.push_back("a::d::e::B");
+ Spec.OldHeader = "foo.h";
+ Spec.OldCC = "foo.cc";
+ Spec.NewHeader = "new_foo.h";
+ Spec.NewCC = "new_foo.cc";
+ auto Results = runClangMoveOnCode(Spec, TestHeader.c_str(), TestCode.c_str());
+ EXPECT_EQ(ExpectedNewCC, Results[Spec.NewCC]);
+ EXPECT_EQ(ExpectedNewHeader, Results[Spec.NewHeader]);
+}
+
} // namespace
} // namespce move
} // namespace clang
More information about the cfe-commits
mailing list