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

Francois Ferrand via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 27 06:04:53 PST 2018


Typz added a comment.

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

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


Repository:
  rC Clang

https://reviews.llvm.org/D43290





More information about the cfe-commits mailing list