[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks

[Daniel Jasper via Phabricator via cfe-commits]

djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Generally looks good.



================
Comment at: lib/Format/TokenAnnotator.cpp:2183
       return 0;
+    if (Left.Previous && Left.Previous->is(tok::equal) &&
+        !Style.Cpp11BracedListStyle)
----------------
Typz wrote:
> djasper wrote:
> > Why is this necessary?
> Otherwise the `PenaltyBreakBeforeFirstCallParameter` is used, which is not consistent with the concept of !Cpp11BraceListStyle (e.g. consider this is an initializer), and gives the following format, which is certainly not the expected result:
> 
>   const std::unordered_map<std::string, int>
>       MyHashTable = { { \"aaaaaaaaaaaaaaaaaaaaa\", 0 },
>                       { \"bbbbbbbbbbbbbbbbbbbbb\", 1 },
>                       { \"ccccccccccccccccccccc\", 2 } };
> 
> (again, the issue only happens when `PenaltyBreakBeforeFirstCallParameter` is increased, e.g. 200 in my case. The default value is 19, so formatting is not affected)
I think PenaltyBreakBeforeFirstCallParameter should not be applied with !Cpp11BracedListStyle whenever a brace is found. Cpp11BracedList style means that braces are to be treated like function calls, but without it, this doesn't make sense. I think this is in some ways better than looking for the "= {".


================
Comment at: unittests/Format/FormatTest.cpp:6655
+  FormatStyle AvoidBreakingFirstArgument = getLLVMStyle();
+  AvoidBreakingFirstArgument.PenaltyBreakBeforeFirstCallParameter = 200;
+  verifyFormat("const std::unordered_map<std::string, int> MyHashTable =\n"
----------------
Typz wrote:
> djasper wrote:
> > How does this penalty influence the test?
> Because breaking after the opening brace is affected by the `PenaltyBreakBeforeFirstCallParameter` : this is an init braced-list, but it is considered as any function.
> 
> Specifically, my patch needs (see prev. comment) to change the penalty for breaking after the brace, but this applies only when `Cpp11BracedListStyle=false`, as per your earlier comment. So this test just verify that the behavior is indeed not affected.
I see.


Repository:
  rC Clang

https://reviews.llvm.org/D43290