Author: Fred Grim Date: 2021-09-28T15:59:37-07:00 New Revision: a36227cb2b6a14fc30bfd303328d1d1abb632010
URL: https://github.com/llvm/llvm-project/commit/a36227cb2b6a14fc30bfd303328d1d1abb632010 DIFF: https://github.com/llvm/llvm-project/commit/a36227cb2b6a14fc30bfd303328d1d1abb632010.diff LOG: 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 Added: Modified: clang/lib/Format/WhitespaceManager.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index a822e0aaf1f93..74136d2f5caa1 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 1c0db59dfd8cf..737cea0291eba 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -17800,13 +17800,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" _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
