[PATCH] D95168: [clang-format] Add InsertBraces option
Björn Schäpers via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 2 12:11:12 PDT 2021
HazardyKnusperkeks added a comment.
In D95168#3102247 <https://reviews.llvm.org/D95168#3102247>, @owenpan wrote:
> In D95168#3100969 <https://reviews.llvm.org/D95168#3100969>, @MyDeveloperDay wrote:
>
>> In D95168#3099920 <https://reviews.llvm.org/D95168#3099920>, @owenpan wrote:
>>
>>> In D95168#3099739 <https://reviews.llvm.org/D95168#3099739>, @MyDeveloperDay wrote:
>>>
>>>> - Look further into possible Removal (I have an idea for how this might be possible, and super useful for LLVM where we don't like single if {} ), I'd like to round out on this before introducing the options rather than having to change them later
>>>>
>>>> - Should we add the possibility of removal should we change the option name to "AutomaticBraces" (thoughts?)
>>>
>>> As mentioned in D95168#3039033 <https://reviews.llvm.org/D95168#3039033>, I think it would be better to handle the removal separately. The LLVM Coding Standards has an entire section <https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements> about this. Some of the listed exceptions/examples there can make things more difficult.
>>
>> I'm thinking more about not adding a "InsertBraces" only later to find it should have been `InsertRemoveBraces` or `AutomaticBraces` i.e. I want to have some idea as to how this might work, if it might be possible even if we land separately.
>
> I think the InsertBraces options can be handled by an `enum`, but the RemoveBraces options most likely will use a `struct`. Does it make sense to have both turned on in the same configuration?
I'm in favor of a struct of enums:
AutomaticBraces:
AfterIf: OnlyIfElse
AfterElse: Remove #this is obviously only to remove, if the else body is a single line
AfterWhile: ...
And we can gradually add new enumerators to handle the delicate nuances of adding or removing the braces.
And we should add them one after another.
================
Comment at: clang/include/clang/Format/Format.h:3696
IndentRequires == R.IndentRequires && IndentWidth == R.IndentWidth &&
- Language == R.Language &&
+ AutomaticBraces == R.AutomaticBraces && Language == R.Language &&
IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
----------------
Resort ;)
================
Comment at: clang/lib/Format/BraceInserter.cpp:105
+ FormatToken *getNext(int &Line, FormatToken *current) {
+ if (Line == 0 && current == nullptr) {
+ return Lines[0]->First;
----------------
Remove the {
Oh the irony. :)
================
Comment at: clang/lib/Format/BraceInserter.h:1
+//===--- BraceInserter.h - Format C++ code --------------------------------===//
+//
----------------
Inserter is misleading, if we want it to be able to remove.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95168/new/
https://reviews.llvm.org/D95168
More information about the cfe-commits
mailing list