summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFred Grim <fgrim@apple.com>2021-09-25 09:07:12 -0700
committerTom Stellard <tstellar@redhat.com>2021-10-27 07:33:42 -0700
commitec4829efcc947bde76d66844b5937f0d7de56058 (patch)
tree5a21effb844b9b70e2551cb16b377f116aed9ade
parent73daeb3d507f7c8da52a35311ec1799f161ac7a5 (diff)
downloadllvm-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.cpp16
-rw-r--r--clang/unittests/Format/FormatTest.cpp14
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"