r198924 - clang-format: Slightly adapt line break in edge case.

dblaikie at gmail.com dblaikie at gmail.com
Sat Jan 11 11:43:21 PST 2014


I believe builders already have a special case - not sure if this would
affect those

Daniel - got a diff of what changes in LLVM between a format of the whole
project with/without this change?
On Sat Jan 11 2014 at 11:39:43 AM, Eric Christopher <echristo at gmail.com>
wrote:

> There's a lot of stuff that looks like this:
>
>       MIB = BuildMI(mainMBB, DL, TII->get(X86::CMOV_GR32), cc)
>               .addReg(cH32).addReg(cL32)
>               .addImm(X86::COND_E);
>
> in the various backends.
>
> -eric
>
>
> On Sat, Jan 11, 2014 at 11:25 AM, dblaikie at gmail.com <dblaikie at gmail.com>
> wrote:
> > If we're voting, I'll put up a hand for the first style (breaking before
> > '.') - but wouldn't mind seeing the real-world examples in LLVM that go
> > either way to get a better sense of how this looks in the real world.
> >
> >
> > On Fri Jan 10 2014 at 11:20:02 AM, Eric Christopher <echristo at gmail.com>
> > wrote:
> >>
> >>
> >>
> >> On Fri Jan 10 2014 at 1:43:37 AM, Daniel Jasper <djasper at google.com>
> >> wrote:
> >>
> >> On Fri, Jan 10, 2014 at 10:07 AM, Chandler Carruth <
> chandlerc at google.com>
> >> wrote:
> >>
> >> On Fri, Jan 10, 2014 at 12:40 AM, Daniel Jasper <djasper at google.com>
> >> wrote:
> >>
> >> Author: djasper
> >> Date: Fri Jan 10 02:40:17 2014
> >> New Revision: 198924
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=198924&view=rev
> >> Log:
> >> clang-format: Slightly adapt line break in edge case.
> >>
> >> Before:
> >>   SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]
> >>       .insert(ccccccccccccccccccccccc);
> >>
> >> After:
> >>   SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)].insert(
> >>       ccccccccccccccccccccccc);
> >>
> >> This seems to be about 3:1 more common in Google and Chromium style and
> I
> >> found
> >> only a handful of instances inside the LLVM codebase.
> >>
> >>
> >> Without any significant numbers in the LLVM codebase, I would prefer
> >> LLVM's style stick to the first -- I think its significantly easier to
> read
> >> personally. I'd also be interested in others' opinions since we don't
> really
> >> have numbers here.
> >>
> >>
> >> Thanks for the additional input. There are less than 10 instances of
> >> either choice in the LLVM codebase providing a bad signal-to-noise
> ratio.
> >> This might mean that it doesn't matter much, but I am happy to switch
> this
> >> back for LLVM style if there is a general preference.
> >>
> >>
> >> Please. The second is much harder to read/scan and is a common pattern
> >> (largely without long lines I guess) in the backend.
> >>
> >> -eric
> >>
> >>
> >>
> >>
> >> Modified:
> >>     cfe/trunk/lib/Format/TokenAnnotator.cpp
> >>     cfe/trunk/unittests/Format/FormatTest.cpp
> >>
> >> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/
> TokenAnnotator.cpp?rev=198924&r1=198923&r2=198924&view=diff
> >>
> >> ============================================================
> ==================
> >> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
> >> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Jan 10 02:40:17 2014
> >> @@ -1203,7 +1203,7 @@ unsigned TokenAnnotator::splitPenalty(co
> >>      return 2;
> >>
> >>    if (Right.isMemberAccess()) {
> >> -    if (Left.isOneOf(tok::r_paren, tok::r_square) && Left.MatchingParen
> >> &&
> >> +    if (Left.is(tok::r_paren) && Left.MatchingParen &&
> >>          Left.MatchingParen->ParameterCount > 0)
> >>        return 20; // Should be smaller than breaking at a nested comma.
> >>      return 150;
> >>
> >> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/
> Format/FormatTest.cpp?rev=198924&r1=198923&r2=198924&view=diff
> >>
> >> ============================================================
> ==================
> >> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> >> +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Jan 10 02:40:17 2014
> >> @@ -3910,8 +3910,8 @@ TEST_F(FormatTest, WrapsAtFunctionCallsI
> >>
> >>    verifyFormat("EXPECT_CALL(SomeObject, SomeFunction(Parameter))\n"
> >>                 "    .WillRepeatedly(Return(SomeValue));");
> >> -  verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]\n"
> >> -               "    .insert(ccccccccccccccccccccccc);");
> >> +  verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa,
> >> bbbbbbbbbbbbbbb)].insert(\n"
> >> +               "    ccccccccccccccccccccccc);");
> >>    verifyFormat("aaaaa(aaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
> >>                 "
> >> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).aaaaa(aaaaa),\n"
> >>                 "      aaaaaaaaaaaaaaaaaaaaa);");
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140111/e55019c0/attachment.html>


More information about the cfe-commits mailing list