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

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

In https://reviews.llvm.org/D43290#1020537, @Typz wrote:

> >> Tweaking the penalty handling would still be needed in any case, since the problem happens also when Cpp11BracedListStyle is true (though the effect is more subtle)
> > 
> > I don't understand this. Which style do you actually care about? With Cpp11BracedListStyle=true or =false?
>
> I use the `Cpp11BracedListStyle=false` style.
>  However, I think the current behavior is wrong also when `Cpp11BracedListStyle=true`. Here the behavior in this case:
>
> Before :
>
>   const std::unordered_map<std::string, int> Something::MyHashTable =
>       {{ "aaaaaaaaaaaaaaaaaaaaa", 0 },
>        { "bbbbbbbbbbbbbbbbbbbbb", 1 },
>        { "ccccccccccccccccccccc", 2 }};
>   
>
> After :
>
>   const std::unordered_set<std::string> Something::MyUnorderedSet = {
>       { "aaaaaaaaaaaaaaaaaaaaa", 0 },
>       { "bbbbbbbbbbbbbbbbbbbbb", 1 },
>       { "ccccccccccccccccccccc", 2 }};


"Before" is the desired behavior. I don't know whether other people have extensively analyzed how to format all the various C++11 braced lists, but we have at Google and think this is good. It is formalized here:
https://google.github.io/styleguide/cppguide.html#Braced_Initializer_List_Format

We prefer breaking before "{" at the higher syntactic level or essentially before the imaginary function call in place of the braced list.

>>> Generally, I think it is better to just rely on penalties, since it gives a way to compare and weight each solution. Then each style can decide what should break first: e.g. a style may also have a lower `PenaltyBreakAssignment`, thus wrapping after the equal sign would be expected...
>> 
>> And with my reasoning, I think exactly the opposite. Penalties are nice, but if the behavior is generally unwanted, than it's very hard to predict in which situations it might still occur.
> 
> Yet on the other hand enforcing too much can lead to cases where the optimizer fails to find a solution, and ends up simply not formatting the line: which is fine is the code is already formatted (but if there were the case we would not need the tool at all :-) )
>  The beauty of penalties is that one can tweak the different penalties so that the behavior is "generally" what would be expected: definitely not predictable, but then there are always cases where the "regular" style does not work, and fallback solutions must be used... (for exemple, I would prefer never to never break after an assignment : yet sometimes it is needed...)
> 
> I can change the patch to disallow breaking, but this would be a slightly more risky change, with possibly more side-effects; and i think that would not solve the issue when `Cpp11BracedListStyle=true`.

It is the better call here. I understand that people that set Cpp11BracedListStyle to false want this behavior and I think it'll always be a surprise when clang-format breaks before the "{". The chance of not coming up with a formatting because of this is somewhat slim. It only adds an additional two characters (we would also not break before the "="). It'd be really weird to only have these exact two line length have a special behavior (slightly shorter - everything fits on one line, slightly longer - a split between type name and variable is necessary).

There is no issue for Cpp11BracedListStyle=true, the behavior is as desired.


Repository:
  rC Clang

https://reviews.llvm.org/D43290