jamesg-nz wrote:

> I think that's not the right way to fix the issue.
> 
> Why are the 2 lines formatted differently? It seems to me that this fixes the 
> symptom, not the cause.

Because for the line where brackets are used it meets the condition in the next 
`if` statement down: 
https://github.com/llvm/llvm-project/blob/7e405eb722e40c79b7726201d0f76b5dab34ba0f/clang/lib/Format/FormatToken.cpp#L122

... and a penalty of zero is returned. Similar exclusion exists for 
precomputing the column formats: 
https://github.com/llvm/llvm-project/blob/7e405eb722e40c79b7726201d0f76b5dab34ba0f/clang/lib/Format/FormatToken.cpp#L191

But this ultimately ends up simply as performance optimization as they're never 
considered later in any case.

Whereas if the braces are used instead of brackets it does not meet the _not a 
left brace_ condition, and gets to the severe penalty return: 
https://github.com/llvm/llvm-project/blob/7e405eb722e40c79b7726201d0f76b5dab34ba0f/clang/lib/Format/FormatToken.cpp#L140
 . This is because suitable column formats are not found, as there was less 
than 5 commas found in the list when precomputing the column formats, and so 
none are generated: 
https://github.com/llvm/llvm-project/blob/7e405eb722e40c79b7726201d0f76b5dab34ba0f/clang/lib/Format/FormatToken.cpp#L273

Seems a little odd to return severe penalty if no column formats are available 
as opposed to having them, but none fit right now.

But I'm thinking maybe better to somehow add a condition that if the left brace 
appears within a short lambda body that'll end up on a single line then a 
penalty of zero is also returned. More/better targeted change.

Ultimately I believe comes down to finding a good justification to return zero 
penalty when the braces are used inside lambda bodies. Maybe affects other 
_short_ functions too like inlines - need to check.

https://github.com/llvm/llvm-project/pull/76675
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to