r290084 - clang-format: Allow "single column" list layout even if that violates the
Tobias Grosser via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 19 00:10:18 PST 2016
On Mon, Dec 19, 2016, at 09:07 AM, Daniel Jasper wrote:
> Yeah, I just saw that when fixing polly format. I'll take a look.
You then probably also saw the second issue:
- Type *Params[] = {PointerType::getUnqual(FunctionType::get(
- Builder.getVoidTy(), Builder.getInt8PtrTy(),
false)),
- Builder.getInt8PtrTy(), Builder.getInt32Ty(),
LongType,
- LongType, LongType};
+ Type *Params[] = {
+ PointerType::getUnqual(FunctionType::get(Builder.getVoidTy(),
Builder.getInt8PtrTy(), false)),
+ Builder.getInt8PtrTy(),
+ Builder.getInt32Ty(),
+ LongType,
+ LongType,
+ LongType};
The lines are now longer than 80 columns, whereas earlier we managed to
remain within 80 columns.
Btw. thank you for updating Polly.
Best,
Tobias
> 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
> >
More information about the cfe-commits
mailing list