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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
