diff options
author | Fred Grim <fgrim@apple.com> | 2021-09-25 09:07:12 -0700 |
---|---|---|
committer | Tom Stellard <tstellar@redhat.com> | 2021-10-27 07:33:42 -0700 |
commit | ec4829efcc947bde76d66844b5937f0d7de56058 (patch) | |
tree | 5a21effb844b9b70e2551cb16b377f116aed9ade | |
parent | 73daeb3d507f7c8da52a35311ec1799f161ac7a5 (diff) | |
download | llvm-ec4829efcc947bde76d66844b5937f0d7de56058.tar.gz |
fixes bug #51926 where dangling comma caused overrun
bug 51926 identified an issue where a dangling comma caused the cell count to be to off by one
Differential Revision: https://reviews.llvm.org/D110481
(cherry picked from commit a36227cb2b6a14fc30bfd303328d1d1abb632010)
-rw-r--r-- | clang/lib/Format/WhitespaceManager.cpp | 16 | ||||
-rw-r--r-- | clang/unittests/Format/FormatTest.cpp | 14 |
2 files changed, 27 insertions, 3 deletions
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index a822e0aaf1f9..74136d2f5caa 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1146,14 +1146,15 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, } else if (C.Tok->is(tok::comma)) { if (!Cells.empty()) Cells.back().EndIndex = i; - Cell++; + if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma + ++Cell; } } else if (Depth == 1) { if (C.Tok == MatchingParen) { if (!Cells.empty()) Cells.back().EndIndex = i; Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr}); - CellCount = Cell + 1; + CellCount = C.Tok->Previous->isNot(tok::comma) ? Cell + 1 : Cell; // Go to the next non-comment and ensure there is a break in front const auto *NextNonComment = C.Tok->getNextNonComment(); while (NextNonComment->is(tok::comma)) @@ -1190,6 +1191,17 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, // So if we split a line previously and the tail line + this token is // less then the column limit we remove the split here and just put // the column start at a space past the comma + // + // FIXME This if branch covers the cases where the column is not + // the first column. This leads to weird pathologies like the formatting + // auto foo = Items{ + // Section{ + // 0, bar(), + // } + // }; + // Well if it doesn't lead to that it's indicative that the line + // breaking should be revisited. Unfortunately alot of other options + // interact with this auto j = i - 1; if ((j - 1) > Start && Changes[j].Tok->is(tok::comma) && Changes[j - 1].NewlinesBefore > 0) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 61a06294c8f9..53afb402d37a 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -17784,13 +17784,25 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) { TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) { auto Style = getLLVMStyle(); Style.AlignArrayOfStructures = FormatStyle::AIAS_Left; + /* FIXME: This case gets misformatted. + verifyFormat("auto foo = Items{\n" + " Section{0, bar(), },\n" + " Section{1, boo() }\n" + "};\n", + Style); + */ + verifyFormat("auto foo = Items{\n" + " Section{\n" + " 0, bar(),\n" + " }\n" + "};\n", + Style); verifyFormat("struct test demo[] = {\n" " {56, 23, \"hello\"},\n" " {-1, 93463, \"world\"},\n" " {7, 5, \"!!\" }\n" "};\n", Style); - verifyFormat("struct test demo[] = {\n" " {56, 23, \"hello\"}, // first line\n" " {-1, 93463, \"world\"}, // second line\n" |