Skip to content

Commit

Permalink
fixes bug llvm#51926 where dangling comma caused overrun
Browse files Browse the repository at this point in the history
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
  • Loading branch information
feg208 committed Sep 28, 2021
1 parent 7c1128f commit a36227c
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
16 changes: 14 additions & 2 deletions clang/lib/Format/WhitespaceManager.cpp
Expand Up @@ -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))
Expand Down Expand Up @@ -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) {
Expand Down
14 changes: 13 additions & 1 deletion clang/unittests/Format/FormatTest.cpp
Expand Up @@ -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"
Expand Down

0 comments on commit a36227c

Please sign in to comment.