summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorYitzhak Mandelbaum <yitzhakm@google.com>2019-09-17 15:10:39 +0000
committerYitzhak Mandelbaum <yitzhakm@google.com>2019-09-17 15:10:39 +0000
commit45b6ca5cd604d9dc0f957a5d80724acd54766f19 (patch)
tree7c21db3ba43e52fe2bc4d33a61c2e552548f7fc7
parent957a6c6bedfa87e5384703724c18ddeada9e189e (diff)
downloadbcm5719-llvm-45b6ca5cd604d9dc0f957a5d80724acd54766f19.tar.gz
bcm5719-llvm-45b6ca5cd604d9dc0f957a5d80724acd54766f19.zip
[clang-format] Fix cleanup of `AnnotatedLine` to include children nodes.
Summary: AnnotatedLine has a tree structure, and things like the body of a lambda will be a child of the lambda expression. For example, [&]() { foo(a); }; will have an AnnotatedLine with a child: [&]() {}; '- foo(a); Currently, when the `Cleaner` class analyzes the affected lines, it does not cleanup the lines' children nodes, which results in missed cleanup opportunities, like the lambda body in the example above. This revision extends the algorithm to visit children, thereby fixing the above problem. Patch by Eric Li. Reviewers: krasimir Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D67659 llvm-svn: 372129
-rw-r--r--clang/lib/Format/Format.cpp29
-rw-r--r--clang/unittests/Format/CleanupTest.cpp9
2 files changed, 25 insertions, 13 deletions
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 81fdab4ed70..801daf11a83 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1417,22 +1417,29 @@ public:
checkEmptyNamespace(AnnotatedLines);
- for (auto &Line : AnnotatedLines) {
- if (Line->Affected) {
- cleanupRight(Line->First, tok::comma, tok::comma);
- cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma);
- cleanupRight(Line->First, tok::l_paren, tok::comma);
- cleanupLeft(Line->First, tok::comma, tok::r_paren);
- cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace);
- cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace);
- cleanupLeft(Line->First, TT_CtorInitializerColon, tok::equal);
- }
- }
+ for (auto *Line : AnnotatedLines)
+ cleanupLine(Line);
return {generateFixes(), 0};
}
private:
+ void cleanupLine(AnnotatedLine *Line) {
+ for (auto *Child : Line->Children) {
+ cleanupLine(Child);
+ }
+
+ if (Line->Affected) {
+ cleanupRight(Line->First, tok::comma, tok::comma);
+ cleanupRight(Line->First, TT_CtorInitializerColon, tok::comma);
+ cleanupRight(Line->First, tok::l_paren, tok::comma);
+ cleanupLeft(Line->First, tok::comma, tok::r_paren);
+ cleanupLeft(Line->First, TT_CtorInitializerComma, tok::l_brace);
+ cleanupLeft(Line->First, TT_CtorInitializerColon, tok::l_brace);
+ cleanupLeft(Line->First, TT_CtorInitializerColon, tok::equal);
+ }
+ }
+
bool containsOnlyComments(const AnnotatedLine &Line) {
for (FormatToken *Tok = Line.First; Tok != nullptr; Tok = Tok->Next) {
if (Tok->isNot(tok::comment))
diff --git a/clang/unittests/Format/CleanupTest.cpp b/clang/unittests/Format/CleanupTest.cpp
index 0628c38a2bd..b0c81b509d2 100644
--- a/clang/unittests/Format/CleanupTest.cpp
+++ b/clang/unittests/Format/CleanupTest.cpp
@@ -183,10 +183,15 @@ TEST_F(CleanupTest, TrailingCommaInParens) {
std::string Code = "int main() { f(,1,,2,3,f(1,2,),4,,);}";
std::string Expected = "int main() { f(1,2,3,f(1,2),4);}";
EXPECT_EQ(Expected, cleanupAroundOffsets({15, 18, 29, 33}, Code));
+
+ // Lambda contents are also checked for trailing commas.
+ Code = "int main() { [](){f(,1,,2,3,f(1,2,),4,,);}();}";
+ Expected = "int main() { [](){f(1,2,3,f(1,2),4);}();}";
+ EXPECT_EQ(Expected, cleanupAroundOffsets({20, 23, 34, 38}, Code));
}
TEST_F(CleanupTest, TrailingCommaInBraces) {
- // Trainling comma is allowed in brace list.
+ // Trailing comma is allowed in brace list.
// If there was trailing comma in the original code, then trailing comma is
// preserved. In this example, element between the last two commas is deleted
// causing the second-last comma to be redundant.
@@ -194,7 +199,7 @@ TEST_F(CleanupTest, TrailingCommaInBraces) {
std::string Expected = "void f() { std::vector<int> v = {1,2,3,}; }";
EXPECT_EQ(Expected, cleanupAroundOffsets({39}, Code));
- // If there was no trailing comma in the original code, then trainling comma
+ // If there was no trailing comma in the original code, then trailing comma
// introduced by replacements should be cleaned up. In this example, the
// element after the last comma is deleted causing the last comma to be
// redundant.
OpenPOWER on IntegriCloud