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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to