[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Not yet done reviewing everything.



================
Comment at: clang/include/clang/Format/Format.h:946
+    /// Remove if there is no comment
+    BIS_RemoveNoComment
+  };
----------------
Maybe differentiate between single line and multi line comments?


================
Comment at: clang/include/clang/Format/Format.h:3675
                R.AlwaysBreakTemplateDeclarations &&
            AttributeMacros == R.AttributeMacros &&
            BinPackArguments == R.BinPackArguments &&
----------------
Add it here


================
Comment at: clang/lib/Format/BraceInserter.cpp:21-56
+bool SupportsAnyAutomaticBraces(FormatStyle &Style, bool insertion) {
+  if (insertion) {
+    if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_Always)
+      return true;
+    if (Style.AutomaticBraces.AfterIf == FormatStyle::BIS_WrapLikely)
+      return true;
+    if (Style.AutomaticBraces.AfterFor == FormatStyle::BIS_Always)
----------------
How about something like that?


================
Comment at: clang/lib/Format/BraceInserter.cpp:114
+
+      switch (ControlFlowType) {
+      case tok::kw_if:
----------------
(Please read for the comment below.) Could also go into a function and use the same mapping.


================
Comment at: clang/lib/Format/BraceInserter.cpp:165
+class NextTokenFetcher {
+  SmallVectorImpl<AnnotatedLine *> &Lines;
+
----------------
Is this supposed to be used?


================
Comment at: clang/lib/Format/BraceInserter.cpp:174
+
+    if (current && current->Next)
+      return current->Next;
----------------
current is not null


================
Comment at: clang/lib/Format/BraceInserter.cpp:178
+    // I'm at the end so I can't take the next
+    if (current && !current->Next) {
+      // I'm at the end of all the lines
----------------
!current->Next also holds always


================
Comment at: clang/lib/Format/BraceInserter.cpp:194
+
+    if (current && !current->Next && Line < (Lines.size() - 1)) {
+      Line++;
----------------
And then this is dead code


================
Comment at: clang/lib/Format/BraceInserter.cpp:211
+    const FormatToken *Next = getNext(LineNumber, RBrace);
+    if (!Next->is(tok2))
+      return false;
----------------
Is there always a next?


================
Comment at: clang/lib/Format/BraceInserter.cpp:284
+    if (LBrace->startsSequence(tok::l_brace, tok::comment)) {
+      if ((StartingToken->is(tok::kw_if) &&
+           Style.AutomaticBraces.AfterIf == FormatStyle::BIS_RemoveNoComment) ||
----------------
This should go into a function, which maybe uses a mapping of the token type to the member used for checking.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95168/new/

https://reviews.llvm.org/D95168