[clang] [clang-format] Don't apply severe penalty if no possible column formats (PR #76675)

James Grant via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 1 18:02:50 PST 2024


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


More information about the cfe-commits mailing list