diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -203,6 +203,17 @@ argument1, argument2); +**AlignArrayOfStructures** (``bool``) + If ``true``, when using initialization for an array + of structs aligns the fields into columns + + .. code-block:: c + struct test demo[] = + { + {56, 23, "hello" }, + {-1, 93463, "world" }, + {7, 5, "!!" } + } **AlignConsecutiveAssignments** (``AlignConsecutiveStyle``) Style of aligning consecutive assignments. diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -244,6 +244,9 @@ accepts ``AllIfsAndElse`` value that allows to put "else if" and "else" short statements on a single line. (Fixes https://llvm.org/PR50019.) +- Option ``AlignArrayOfStructure`` has been added to allow for ordering array-like + initializers. + libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -90,6 +90,18 @@ /// brackets. BracketAlignmentStyle AlignAfterOpenBracket; + /// if ``true``, when using initialization for an array of structs + /// aligns the fields into columns + /// \code + /// struct test demo[] = + /// { + /// {56, 23, "hello" }, + /// {-1, 93463, "world" }, + /// {7, 5, "!!" } + /// } + /// \endcode + bool AlignArrayOfStructures; + /// Styles for alignment of consecutive tokens. Tokens can be assignment signs /// (see /// ``AlignConsecutiveAssignments``), bitfield member separators (see @@ -3249,6 +3261,7 @@ bool operator==(const FormatStyle &R) const { return AccessModifierOffset == R.AccessModifierOffset && AlignAfterOpenBracket == R.AlignAfterOpenBracket && + AlignArrayOfStructures == R.AlignArrayOfStructures && AlignConsecutiveAssignments == R.AlignConsecutiveAssignments && AlignConsecutiveBitFields == R.AlignConsecutiveBitFields && AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -506,6 +506,7 @@ IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset); IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket); + IO.mapOptional("AlignArrayOfStructures", Style.AlignArrayOfStructures); IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros); IO.mapOptional("AlignConsecutiveAssignments", Style.AlignConsecutiveAssignments); @@ -941,6 +942,7 @@ LLVMStyle.AccessModifierOffset = -2; LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right; LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align; + LLVMStyle.AlignArrayOfStructures = false; LLVMStyle.AlignOperands = FormatStyle::OAS_Align; LLVMStyle.AlignTrailingComments = true; LLVMStyle.AlignConsecutiveAssignments = FormatStyle::ACS_None; diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -10,7 +10,11 @@ #include "NamespaceEndCommentsFixer.h" #include "WhitespaceManager.h" #include "llvm/Support/Debug.h" +#include +#include #include +#include +#include #define DEBUG_TYPE "format-formatter" @@ -26,6 +30,54 @@ NextNext && NextNext->is(tok::l_brace); } +// The notion here is that we walk through the annotated line looking for +// things like the initialization of arrays and flag them. +bool isArrayOfStructures(const AnnotatedLine &Line) { + if (!Line.MustBeDeclaration || Line.First == Line.Last) + return false; + // So the method to detect array-like initializers is to look + // for balanced sets of braces separated by commas wrapped by + // an outer set of braces + std::stack Braces; + int Depth = 0; + for (const auto *CurrentToken = Line.First; + CurrentToken != Line.Last && CurrentToken != nullptr; + CurrentToken = CurrentToken->getNextNonComment()) { + if (CurrentToken->is(tok::l_brace)) { + if (++Depth <= 2) + Braces.push(CurrentToken); + } else if (CurrentToken->is(tok::r_brace)) { + if (--Depth < 2) + Braces.push(CurrentToken); + } else if (CurrentToken->is(tok::comma)) { + if (Depth == 1) + Braces.push(CurrentToken); + } + } + if (Depth != 0 || Braces.empty()) + return false; + unsigned Commas = 0U; + unsigned RBraces = 0U; + while (!Braces.empty()) { + const auto *CurrentToken = Braces.top(); + if (CurrentToken->is(tok::r_brace)) { + if (--Depth < -2) + return false; + ++RBraces; + } else if (CurrentToken->is(tok::l_brace)) { + if (++Depth > 0) + return false; + } else if (CurrentToken->is(tok::comma)) { + ++Commas; + } + Braces.pop(); + } + if (RBraces > 0) + --RBraces; + return (RBraces == Commas || (Commas == (RBraces - 1))) && + (Commas > 0 && Depth == 0); +} + /// Tracks the indent level of \c AnnotatedLines across levels. /// /// \c nextLine must be called for each \c AnnotatedLine, after which \c @@ -779,7 +831,7 @@ LineFormatter(ContinuationIndenter *Indenter, WhitespaceManager *Whitespaces, const FormatStyle &Style, UnwrappedLineFormatter *BlockFormatter) - : Indenter(Indenter), Whitespaces(Whitespaces), Style(Style), + : Indenter(Indenter), Style(Style), Whitespaces(Whitespaces), BlockFormatter(BlockFormatter) {} virtual ~LineFormatter() {} @@ -866,10 +918,10 @@ } ContinuationIndenter *Indenter; + const FormatStyle &Style; private: WhitespaceManager *Whitespaces; - const FormatStyle &Style; UnwrappedLineFormatter *BlockFormatter; }; @@ -1101,6 +1153,176 @@ llvm::SpecificBumpPtrAllocator Allocator; }; +/// Breaks a static array of struct initializers into regular +/// columns +class AlignedArrayOfStructuresLineFormatter : public LineFormatter { + struct FormatArgs { + LineState &State; + unsigned &Penalty; + unsigned FirstIndent; + unsigned FirstStartColumn; + bool DryRun; + }; + +public: + AlignedArrayOfStructuresLineFormatter(ContinuationIndenter *Indenter, + WhitespaceManager *Whitespaces, + const FormatStyle &Style, + UnwrappedLineFormatter *BlockFormatter) + : LineFormatter(Indenter, Whitespaces, Style, BlockFormatter) {} + + /// Formats the line by finding line breaks with line lengths + /// below the column limit that still orders the array initializers + /// into tidy columns + unsigned formatLine(const AnnotatedLine &Line, unsigned FirstIndent, + unsigned FirstStartColumn, bool DryRun) override { + unsigned Penalty = 0; + auto LayoutMatrix = getLayoutMatrix(Line); + LineState State = + Indenter->getInitialState(FirstIndent, FirstStartColumn, &Line, DryRun); + unsigned Depth = 0; + while (State.NextToken) { + auto *CurrentToken = State.NextToken->Previous; + if (CurrentToken->is(tok::l_brace)) { + FormatArgs Args{State, Penalty, FirstIndent, FirstStartColumn, DryRun}; + enterInitializer(LayoutMatrix, Args, Depth + 1); + } else { + skipToken(State, Penalty, DryRun); + } + } + return Penalty; + } + +private: + using Matrix = std::vector>; + + // Whatever the newline situation is with this line keep it and move on + void skipToken(LineState &State, unsigned &Penalty, bool DryRun, + unsigned ExtraSpaces = 0) { + bool Newline = + Indenter->mustBreak(State) || + (Indenter->canBreak(State) && State.NextToken->NewlinesBefore > 0); + formatChildren(State, Newline, DryRun, Penalty); + Indenter->addTokenToState(State, Newline, DryRun, ExtraSpaces); + } + + // Break the line + void insertBreak(LineState &State, unsigned &Penalty, bool DryRun) { + formatChildren(State, true, DryRun, Penalty); + Indenter->addTokenToState(State, true, DryRun); + } + + void enterInitializer(const Matrix &Layout, FormatArgs Args, unsigned Depth) { + if (Depth == 1) { + insertBreak(Args.State, Args.Penalty, Args.DryRun); + return enterInitializer(Layout, Args, Depth + 1); + } + unsigned Row = 0U; + unsigned Column = 0U; + while (Args.State.NextToken) { + auto *CurrentToken = Args.State.NextToken->Previous; + if (CurrentToken->is(tok::l_brace)) { + ++Depth; + } else if (CurrentToken->is(tok::r_brace)) { + --Depth; + } + + if (CurrentToken->is(tok::r_brace)) { + if (Depth == 2) { + if (Args.State.NextToken->is(tok::r_brace)) { + insertBreak(Args.State, Args.Penalty, Args.DryRun); + } else { + skipToken(Args.State, Args.Penalty, Args.DryRun); + insertBreak(Args.State, Args.Penalty, Args.DryRun); + } + Column = 0; + ++Row; + } else { + skipToken(Args.State, Args.Penalty, Args.DryRun); + } + } else if (CurrentToken->is(tok::comma) && Depth == 3) { + skipToken(Args.State, Args.Penalty, Args.DryRun, Layout[Row][Column++]); + } else { + if (Args.State.NextToken->is(tok::r_brace) && Depth == 3) { + skipToken(Args.State, Args.Penalty, Args.DryRun, + Layout[Row][Column++] + 1U); + } else { + skipToken(Args.State, Args.Penalty, Args.DryRun); + } + } + } + } + + const FormatToken *enterInitializer(const FormatToken *CurrentToken, + Matrix &Layout, const AnnotatedLine &Line, + unsigned Depth) { + if (Depth < 2) { + while (CurrentToken != Line.Last) { + if (CurrentToken->is(tok::l_brace)) { + CurrentToken = enterInitializer(CurrentToken->getNextNonComment(), + Layout, Line, Depth + 1); + } else { + CurrentToken = CurrentToken->getNextNonComment(); + } + } + return CurrentToken; + } + Layout.emplace_back(); + auto Row = Layout.size() - 1; + Layout[Row].push_back(1); + auto Column = 0U; + while (CurrentToken != Line.Last) { + if (CurrentToken->is(tok::l_brace)) { + Depth++; + } else if (CurrentToken->is(tok::r_brace)) { + if (Depth == 2) + return CurrentToken->getNextNonComment(); + Depth--; + } + if (CurrentToken->is(tok::comma)) { + Column++; + Layout[Row].push_back(1); + } else { + auto NewValue = Layout[Row][Column] + CurrentToken->ColumnWidth; + if (Style.ColumnLimit > 0 && NewValue > Style.ColumnLimit) + Layout[Row][Column] = CurrentToken->ColumnWidth + 1; + else + Layout[Row][Column] += CurrentToken->ColumnWidth; + } + CurrentToken = CurrentToken->getNextNonComment(); + } + return CurrentToken; + } + + Matrix getLayoutMatrix(const AnnotatedLine &Line) { + Matrix LayoutMatrix{}; + const auto *CurrentToken = Line.First; + unsigned Depth = 0; + // First step get the Column widths + while (CurrentToken != Line.Last) { + if (CurrentToken->is(tok::l_brace)) { + CurrentToken = enterInitializer(CurrentToken->getNextNonComment(), + LayoutMatrix, Line, ++Depth); + } else { + CurrentToken = CurrentToken->getNextNonComment(); + } + } + // Now adjust the values at each column to just contain the number + // of extra spaces to add + assert(LayoutMatrix.size() > 0); + for (auto Column = 0U; Column < LayoutMatrix[0].size(); ++Column) { + auto MaxColumn = 0U; + for (const auto &Row : LayoutMatrix) { + MaxColumn = std::max(MaxColumn, Row[Column]); + } + for (auto &Row : LayoutMatrix) { + Row[Column] = MaxColumn - Row[Column]; + } + } + return LayoutMatrix; + } +}; + } // anonymous namespace unsigned UnwrappedLineFormatter::format( @@ -1171,7 +1393,14 @@ !Style.JavaScriptWrapImports)) || (Style.isCSharp() && TheLine.InPPDirective); // don't split #regions in C# - if (Style.ColumnLimit == 0) + bool AlignArrayOfStructures = + (Style.AlignArrayOfStructures && isArrayOfStructures(TheLine)); + if (AlignArrayOfStructures) { + Penalty += AlignedArrayOfStructuresLineFormatter(Indenter, Whitespaces, + Style, this) + .formatLine(TheLine, NextStartColumn + Indent, + FirstLine ? FirstStartColumn : 0, DryRun); + } else if (Style.ColumnLimit == 0) NoColumnLimitLineFormatter(Indenter, Whitespaces, Style, this) .formatLine(TheLine, NextStartColumn + Indent, FirstLine ? FirstStartColumn : 0, DryRun); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -15,6 +15,8 @@ #include "llvm/Support/MemoryBuffer.h" #include "gtest/gtest.h" +#include + #define DEBUG_TYPE "format-test" using clang::tooling::ReplacementTest; @@ -16347,6 +16349,147 @@ getLLVMStyle()); } +template +auto createStylesImpl( + const std::array &ACSValues, + const std::array &Limits) { + std::array Styles; + auto StyleIter = Styles.begin(); + for (const auto &OuterACS : ACSValues) { + for (const auto &InnerACS : ACSValues) { + for (const auto &LimitValue : Limits) { + auto &StyleValue = *(StyleIter); + StyleValue = getLLVMStyle(); + StyleValue.AlignArrayOfStructures = true; + StyleValue.AlignConsecutiveAssignments = OuterACS; + StyleValue.AlignConsecutiveDeclarations = InnerACS; + StyleValue.ColumnLimit = LimitValue; + ++StyleIter; + } + } + } + return Styles; +} + +auto createStyles() { + constexpr static auto N = 5; + std::array ACSValues{ + {FormatStyle::AlignConsecutiveStyle::ACS_None, + FormatStyle::AlignConsecutiveStyle::ACS_Consecutive, + FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLines, + FormatStyle::AlignConsecutiveStyle::ACS_AcrossComments, + FormatStyle::AlignConsecutiveStyle::ACS_AcrossEmptyLinesAndComments}}; + std::array LimitValues{{0, 80}}; + return createStylesImpl(ACSValues, LimitValues); +} + +TEST_F(FormatTest, CatchAlignArrayOfStructures) { + const static auto Styles = createStyles(); + for (const auto &Style : Styles) { + verifyFormat("struct test demo[] = {\n" + " {56, 23, \"hello\" },\n" + " {-1, 93463, \"world\" },\n" + " {7, 5, \"!!\" }\n" + "};\n", + Style); + verifyFormat("struct test demo[3] = {\n" + " {56, 23, \"hello\" },\n" + " {-1, 93463, \"world\" },\n" + " {7, 5, \"!!\" }\n" + "};\n", + Style); + verifyFormat("struct test demo[3] = {\n" + " {int{56}, 23, \"hello\" },\n" + " {int{-1}, 93463, \"world\" },\n" + " {int{7}, 5, \"!!\" }\n" + "};\n", + Style); + verifyFormat("struct test demo[] = {\n" + " {56, 23, \"hello\" },\n" + " {-1, 93463, \"world\" },\n" + " {7, 5, \"!!\" },\n" + "};\n", + Style); + verifyFormat("test demo[] = {\n" + " {56, 23, \"hello\" },\n" + " {-1, 93463, \"world\" },\n" + " {7, 5, \"!!\" },\n" + "};\n", + Style); + verifyFormat("demo = std::array{\n" + " test{56, 23, \"hello\" },\n" + " test{-1, 93463, \"world\" },\n" + " test{7, 5, \"!!\" },\n" + "};\n", + Style); + verifyFormat("test demo[] = {\n" + " {56, 23, \"hello\" },\n" + "#if X\n" + " {-1, 93463, \"world\" },\n" + "#endif\n" + " {7, 5, \"!!\" }\n" + "};\n", + Style); + } + auto Style = getLLVMStyle(); + Style.AlignArrayOfStructures = true; + verifyFormat( + "test demo[] = {\n" + " {56, 23, \"hello world i am a very long line that really, in any " + "\"\n" + " \"just world, ought to be split over multiple lines\" " + "},\n" + " {-1, 93463, \"world\" " + "},\n" + " {7, 5, \"!!\" " + "},\n" + "};\n", + Style); + Style.ColumnLimit = 0; + EXPECT_EQ( + "test demo[] = {\n" + " {56, 23, \"hello world i am a very long line that really, in any " + "just world, ought to be split over multiple lines\" },\n" + " {-1, 93463, \"world\" " + " },\n" + " {7, 5, \"!!\" " + " },\n" + "};", + format("test demo[] = {{56, 23, \"hello world i am a very long line " + "that really, in any just world, ought to be split over multiple " + "lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};", + Style)); + Style.ColumnLimit = 20; + EXPECT_EQ( + "test demo[] = {\n" + " {56, 23, \"hello world i am a very long line that really, in any " + "just world, ought to be split over multiple lines\" },\n" + " {-1, 93463, \"world\" " + " },\n" + " {7, 5, \"!!\" " + " },\n" + "};", + format("test demo[] = {{56, 23, \"hello world i am a very long line " + "that really, in any just world, ought to be split over multiple " + "lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};", + Style)); + + Style.ColumnLimit = 100; + EXPECT_EQ( + "test demo[] = {\n" + " {56, 23, \"hello world i am a very long line that really, in any " + "just world, ought to be split over multiple lines\" },\n" + " {-1, 93463, \"world\" " + " },\n" + " {7, 5, \"!!\" " + " },\n" + "};", + format("test demo[] = {{56, 23, \"hello world i am a very long line " + "that really, in any just world, ought to be split over multiple " + "lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};", + Style)); +} + TEST_F(FormatTest, UnderstandsPragmas) { verifyFormat("#pragma omp reduction(| : var)"); verifyFormat("#pragma omp reduction(+ : var)");