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

Eric Christopher echristo at gmail.com
Sat Jan 11 11:39:43 PST 2014


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
>>
>>
>



More information about the cfe-commits mailing list