r290084 - clang-format: Allow "single column" list layout even if that violates the

[Tobias Grosser via cfe-commits]

Hi Daniel,

this commit introduce an unnecessary string split, which does not seem
to be an intended result of the formatting style change this commit
introduced:

BEFORE:

#define SCOP_STAT(NAME, DESC)

llvm::Statistic RejectStatistics[] = {
    SCOP_STAT(CFG, ""),
    SCOP_STAT(InvalidTerminator, "Unsupported terminator instruction"),
    SCOP_STAT(IrreducibleRegion, "Irreducible loops"),
    SCOP_STAT(UndefCond, "Undefined branch condition"),
    SCOP_STAT(NonSimpleMemoryAccess,
              "Compilated access semantics (volatile or atomic)"),
};

AFTER:

#define SCOP_STAT(NAME, DESC)

llvm::Statistic RejectStatistics[] = {
    SCOP_STAT(CFG, ""),
    SCOP_STAT(InvalidTerminator, "Unsupported terminator instruction"),
    SCOP_STAT(IrreducibleRegion, "Irreducible loops"),
    SCOP_STAT(UndefCond, "Undefined branch condition"),
    SCOP_STAT(NonSimpleMemoryAccess, "Compilated access semantics
    (volatile or "
                                     "atomic)"

As this worked before, this seems to be a regression.

Best,
Tobias

On Mon, Dec 19, 2016, at 08:26 AM, Daniel Jasper via cfe-commits wrote:
> Author: djasper
> Date: Mon Dec 19 01:26:11 2016
> New Revision: 290084
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290084&view=rev
> Log:
> clang-format: Allow "single column" list layout even if that violates the
> column limit.
> 
> Single-column layout basically means that we format the list with one
> element per line. Not doing that when there is a column limit violation
> doesn't change the fact that there is an item that doesn't fit within
> the column limit.
> 
> Before (with a column limit of 30):
>   std::vector<int> a = {
>       aaaaaaaa, aaaaaaaa,
>       aaaaaaaa, aaaaaaaa,
>       aaaaaaaaaa, aaaaaaaa,
>       aaaaaaaaaaaaaaaaaaaaaaaaaaa};
> 
> After:
>   std::vector<int> a = {
>       aaaaaaaa,
>       aaaaaaaa,
>       aaaaaaaa,
>       aaaaaaaa,
>       aaaaaaaaaa,
>       aaaaaaaa,
>       aaaaaaaaaaaaaaaaaaaaaaaaaaa};
> 
> (and previously we would have formatted like "After" it wasn't for the
> one
> item that is too long)
> 
> Modified:
>     cfe/trunk/lib/Format/FormatToken.cpp
>     cfe/trunk/unittests/Format/FormatTest.cpp
> 
> Modified: cfe/trunk/lib/Format/FormatToken.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.cpp?rev=290084&r1=290083&r2=290084&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/FormatToken.cpp (original)
> +++ cfe/trunk/lib/Format/FormatToken.cpp Mon Dec 19 01:26:11 2016
> @@ -273,7 +273,7 @@ void CommaSeparatedList::precomputeForma
>        continue;
>  
>      // Ignore layouts that are bound to violate the column limit.
> -    if (Format.TotalWidth > Style.ColumnLimit)
> +    if (Format.TotalWidth > Style.ColumnLimit && Columns > 1)
>        continue;
>  
>      Formats.push_back(Format);
> @@ -287,7 +287,7 @@ CommaSeparatedList::getColumnFormat(unsi
>             I = Formats.rbegin(),
>             E = Formats.rend();
>         I != E; ++I) {
> -    if (I->TotalWidth <= RemainingCharacters) {
> +    if (I->TotalWidth <= RemainingCharacters || I->Columns == 1) {
>        if (BestFormat && I->LineCount > BestFormat->LineCount)
>          break;
>        BestFormat = &*I;
> 
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=290084&r1=290083&r2=290084&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Dec 19 01:26:11 2016
> @@ -6779,6 +6779,18 @@ TEST_F(FormatTest, FormatsBracedListsInC
>        "                          1, 22, 333, 4444, 55555, 666666,
>        7777777,\n"
>        "                          1, 22, 333, 4444, 55555, 666666,
>        7777777,\n"
>        "                          1, 22, 333, 4444, 55555, 666666,
>        7777777});");
> +
> +  // Allow "single-column" layout even if that violates the column
> limit. There
> +  // isn't going to be a better way.
> +  verifyFormat("std::vector<int> a = {\n"
> +               "    aaaaaaaa,\n"
> +               "    aaaaaaaa,\n"
> +               "    aaaaaaaa,\n"
> +               "    aaaaaaaa,\n"
> +               "    aaaaaaaaaa,\n"
> +               "    aaaaaaaa,\n"
> +               "    aaaaaaaaaaaaaaaaaaaaaaaaaaa};",
> +               getLLVMStyleWithColumns(30));
>  }
>  
>  TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits