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

[Daniel Jasper via cfe-commits]

Yeah, I just saw that when fixing polly format. I'll take a look.

On Mon, Dec 19, 2016 at 9:05 AM, Tobias Grosser <tobias at grosser.es> wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161219/b907aaac/attachment-0001.html>