[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGadfe58b09df9: [clang-format] Minimize the damage caused by AlignArrayOfStructures when… (authored by MyDeveloperDay). Repository: rG LLVM Github M

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay updated this revision to Diff 414857. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Fixing final review comments prior to commit CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

owenpan added inline comments. Comment at: clang/lib/Format/WhitespaceManager.h:321 + } auto Start = (CellStart + RowCount * CellCount); auto End = Start + Offset; MyDeveloperDay wrote: > Ultimately these calculations are incorrect unless every

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

owenpan added inline comments. Comment at: clang/lib/Format/WhitespaceManager.h:318 Next = Next->NextColumnElement) { + if (RowCount > MaxRowCount) { +break; owenpan wrote: > curdeius wrote: > > You can elide braces. Personally I use Remove

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

owenpan added a comment. In D121069#3362424 , @MyDeveloperDay wrote: > NOTE: I've tried to collate all the reported crashing examples and run this > fix through them (both Left and Right) all pass except this one below > > > > void foo() > { > aut

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

owenpan added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:1073 + auto *Start = + (Cells.begin() + RowCount * CellDescs.CellCounts[RowCount]); auto *End = Start + Offset; Can we use `CellCounts[0]` instead?

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

feg208 added a comment. In D121069#3362450 , @MyDeveloperDay wrote: > @feg208 I could do with some clarity on the algorithm, am I correct in > thinking it requires that the first row, to contain at least the maximum > number of columns for the rest of

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.h:318 Next = Next->NextColumnElement) { + if (RowCount > MaxRowCount) { +break; You can elide braces. Personally I use RemoveBraces locally. CHANGES SINCE LAST

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay added a comment. In D121069#3362448 , @feg208 wrote: >> This seems like a reasonable compromise I kind of feel this might be the best initial option, I don't want to disable it completely because it looks like people are trying to use it,

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay added a comment. @feg208 I could do with some clarity on the algorithm, am I correct in thinking it requires that the first row, contain at least the maximum number of columns for the rest of the array. I sort of noticed it was fine if the first row was larger { {1,2,3 }

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

feg208 accepted this revision. feg208 added a comment. I have been watching the issues pile up around this but work has prevented me from focusing on them. This seems like a reasonable compromise but the note of disabling this feature is also reasonable. Given where it had to be placed in the f

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:1118 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces; ++CellIter; + for (auto i = 1U; i < CellDescs.CellCounts[0]; i++, ++CellIter) { I don't understand in

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay updated this revision to Diff 413301. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. There were no checks for if the cell would run off the end of the CellDescs structure, this is part of the reason for the crashes that the PrevIter->Index becomes

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. Nits only. +1 for adding a note in the option documentation. Comment at: clang/lib/Format/WhitespaceManager.h:203 +// Determine if every row in the the array +//

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay added a comment. NOTE: I've tried to collate all the reported crashing examples and run this fix through them (both Left and Right) all pass except this one below void foo() { auto thing = test{ { {something}, //A } }; } This is not related to i

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay updated this revision to Diff 413299. MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added a comment. Lets get the comment aligned with the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

HazardyKnusperkeks added a comment. There should also be a warning or at least note on the option description about the current limitations. Comment at: clang/lib/Format/WhitespaceManager.h:202-203 + +// Determine if the array is "Square" i.e. every row has the same number

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay updated this revision to Diff 413298. MyDeveloperDay added a comment. Fix review comments and clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib/Format/WhitespaceManager.cpp clang/lib/Format/Whites

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.h:204 +// of columns as the first row +bool IsSquare() { + if (CellCounts.empty()) Don't really like the name... Some suggestions: - HasEqualRows - HasEqualRowLengths - Ha

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay updated this revision to Diff 413288. MyDeveloperDay added a comment. Add more producers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib/Format/WhitespaceManager.cpp clang/lib/Format/WhitespaceManager.h cl

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay updated this revision to Diff 413286. MyDeveloperDay added a comment. Update to always use the first row number of columns or unit tests crash CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib/Format/WhitespaceM

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan. MyDeveloperDay added projects: clang, clang-format. Herald added a project: All. MyDeveloperDay requested review of this revision. I have lost count of the number of times this has been rep