[clang-format][NFC] Code Tidies in UnwrappedLineFormatter
authorBjörn Schäpers <bjoern@hazardy.de>
Fri, 3 Dec 2021 07:13:57 +0000 (08:13 +0100)
committerBjörn Schäpers <bjoern@hazardy.de>
Thu, 3 Feb 2022 21:55:27 +0000 (22:55 +0100)
* Give I[1] and I[-1] a name:
  - Easier to understand
  - Easier to debug (since you don't go through operator[] everytime)
* TheLine->First != TheLine->Last follows since last is a l brace and
  first isn't.
* Factor the check for is(tok::l_brace) out.
* Drop else after return.

Differential Revision: https://reviews.llvm.org/D115060

clang/lib/Format/UnwrappedLineFormatter.cpp

index f7d26b8..9f49410 100644 (file)
@@ -228,10 +228,11 @@ private:
     const AnnotatedLine *TheLine = *I;
     if (TheLine->Last->is(TT_LineComment))
       return 0;
-    if (I[1]->Type == LT_Invalid || I[1]->First->MustBreakBefore)
+    const auto &NextLine = *I[1];
+    if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)
       return 0;
     if (TheLine->InPPDirective &&
-        (!I[1]->InPPDirective || I[1]->First->HasUnescapedNewline))
+        (!NextLine.InPPDirective || NextLine.First->HasUnescapedNewline))
       return 0;
 
     if (Style.ColumnLimit > 0 && Indent > Style.ColumnLimit)
@@ -248,15 +249,16 @@ private:
     if (TheLine->Last->is(TT_FunctionLBrace) &&
         TheLine->First == TheLine->Last &&
         !Style.BraceWrapping.SplitEmptyFunction &&
-        I[1]->First->is(tok::r_brace))
+        NextLine.First->is(tok::r_brace))
       return tryMergeSimpleBlock(I, E, Limit);
 
-    // Handle empty record blocks where the brace has already been wrapped
-    if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last &&
-        I != AnnotatedLines.begin()) {
-      bool EmptyBlock = I[1]->First->is(tok::r_brace);
+    const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
+    // Handle empty record blocks where the brace has already been wrapped.
+    if (PreviousLine && TheLine->Last->is(tok::l_brace) &&
+        TheLine->First == TheLine->Last) {
+      bool EmptyBlock = NextLine.First->is(tok::r_brace);
 
-      const FormatToken *Tok = I[-1]->First;
+      const FormatToken *Tok = PreviousLine->First;
       if (Tok && Tok->is(tok::comment))
         Tok = Tok->getNextNonComment();
 
@@ -278,48 +280,44 @@ private:
         return 0;
     }
 
-    auto ShouldMergeShortFunctions =
-        [this, B = AnnotatedLines.begin()](
-            SmallVectorImpl<AnnotatedLine *>::const_iterator I) {
-          if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
-            return true;
-          if (Style.AllowShortFunctionsOnASingleLine >=
-                  FormatStyle::SFS_Empty &&
-              I[1]->First->is(tok::r_brace))
-            return true;
-
-          if (Style.AllowShortFunctionsOnASingleLine &
-              FormatStyle::SFS_InlineOnly) {
-            // Just checking TheLine->Level != 0 is not enough, because it
-            // provokes treating functions inside indented namespaces as short.
-            if (Style.isJavaScript() && (*I)->Last->is(TT_FunctionLBrace))
-              return true;
-
-            if ((*I)->Level != 0) {
-              if (I == B)
-                return false;
-
-              // TODO: Use IndentTracker to avoid loop?
-              // Find the last line with lower level.
-              auto J = I - 1;
-              for (; J != B; --J)
-                if ((*J)->Level < (*I)->Level)
-                  break;
-
-              // Check if the found line starts a record.
-              for (const FormatToken *RecordTok = (*J)->Last; RecordTok;
-                   RecordTok = RecordTok->Previous)
-                if (RecordTok->is(tok::l_brace))
-                  return RecordTok->is(TT_RecordLBrace);
-
-              return false;
-            }
-          }
+    auto ShouldMergeShortFunctions = [this, &I, &NextLine, PreviousLine,
+                                      TheLine]() {
+      if (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All)
+        return true;
+      if (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+          NextLine.First->is(tok::r_brace))
+        return true;
+
+      if (Style.AllowShortFunctionsOnASingleLine &
+          FormatStyle::SFS_InlineOnly) {
+        // Just checking TheLine->Level != 0 is not enough, because it
+        // provokes treating functions inside indented namespaces as short.
+        if (Style.isJavaScript() && TheLine->Last->is(TT_FunctionLBrace))
+          return true;
+
+        if (TheLine->Level != 0) {
+          if (!PreviousLine)
+            return false;
+
+          // TODO: Use IndentTracker to avoid loop?
+          // Find the last line with lower level.
+          auto J = I - 1;
+          for (; J != AnnotatedLines.begin(); --J)
+            if ((*J)->Level < TheLine->Level)
+              break;
+
+          // Check if the found line starts a record.
+          for (const FormatToken *RecordTok = (*J)->Last; RecordTok;
+               RecordTok = RecordTok->Previous)
+            if (RecordTok->is(tok::l_brace))
+              return RecordTok->is(TT_RecordLBrace);
+        }
+      }
 
-          return false;
-        };
+      return false;
+    };
 
-    bool MergeShortFunctions = ShouldMergeShortFunctions(I);
+    bool MergeShortFunctions = ShouldMergeShortFunctions();
 
     if (Style.CompactNamespaces) {
       if (auto nsToken = TheLine->First->getNamespaceToken()) {
@@ -330,7 +328,7 @@ private:
                closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
                I[i + 1]->Last->TotalLength < Limit;
              i++, --closingLine) {
-          // No extra indent for compacted namespaces
+          // No extra indent for compacted namespaces.
           IndentTracker.skipLine(*I[i + 1]);
 
           Limit -= I[i + 1]->Last->TotalLength;
@@ -346,20 +344,20 @@ private:
                    getMatchingNamespaceTokenText(I[i + 1], AnnotatedLines) &&
                openingLine == I[i + 1]->MatchingOpeningBlockLineIndex;
              i++, --openingLine) {
-          // No space between consecutive braces
+          // No space between consecutive braces.
           I[i + 1]->First->SpacesRequiredBefore = !I[i]->Last->is(tok::r_brace);
 
-          // Indent like the outer-most namespace
+          // Indent like the outer-most namespace.
           IndentTracker.nextLine(*I[i + 1]);
         }
         return i;
       }
     }
 
-    // Try to merge a function block with left brace unwrapped
+    // Try to merge a function block with left brace unwrapped.
     if (TheLine->Last->is(TT_FunctionLBrace) && TheLine->First != TheLine->Last)
       return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
-    // Try to merge a control statement block with left brace unwrapped
+    // Try to merge a control statement block with left brace unwrapped.
     if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
         TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
                                 TT_ForEachMacro)) {
@@ -367,67 +365,77 @@ private:
                  ? tryMergeSimpleBlock(I, E, Limit)
                  : 0;
     }
-    // Try to merge a control statement block with left brace wrapped
-    if (I[1]->First->is(tok::l_brace) &&
-        (TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
-                                 tok::kw_for, tok::kw_switch, tok::kw_try,
-                                 tok::kw_do, TT_ForEachMacro) ||
-         (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
-          TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
-        Style.BraceWrapping.AfterControlStatement ==
-            FormatStyle::BWACS_MultiLine) {
-      // If possible, merge the next line's wrapped left brace with the current
-      // line. Otherwise, leave it on the next line, as this is a multi-line
-      // control statement.
-      return (Style.ColumnLimit == 0 ||
-              TheLine->Last->TotalLength <= Style.ColumnLimit)
-                 ? 1
-                 : 0;
-    } else if (I[1]->First->is(tok::l_brace) &&
-               TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
-                                       tok::kw_for, TT_ForEachMacro)) {
-      return (Style.BraceWrapping.AfterControlStatement ==
-              FormatStyle::BWACS_Always)
-                 ? tryMergeSimpleBlock(I, E, Limit)
-                 : 0;
-    } else if (I[1]->First->is(tok::l_brace) &&
-               TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
-               Style.BraceWrapping.AfterControlStatement ==
-                   FormatStyle::BWACS_MultiLine) {
-      // This case if different from the upper BWACS_MultiLine processing
-      // in that a preceding r_brace is not on the same line as else/catch
-      // most likely because of BeforeElse/BeforeCatch set to true.
-      // If the line length doesn't fit ColumnLimit, leave l_brace on the
-      // next line to respect the BWACS_MultiLine.
-      return (Style.ColumnLimit == 0 ||
-              TheLine->Last->TotalLength <= Style.ColumnLimit)
-                 ? 1
-                 : 0;
+    // Try to merge a control statement block with left brace wrapped.
+    if (NextLine.First->is(tok::l_brace)) {
+      if ((TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
+                                   tok::kw_for, tok::kw_switch, tok::kw_try,
+                                   tok::kw_do, TT_ForEachMacro) ||
+           (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
+            TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
+          Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_MultiLine) {
+        // If possible, merge the next line's wrapped left brace with the
+        // current line. Otherwise, leave it on the next line, as this is a
+        // multi-line control statement.
+        return (Style.ColumnLimit == 0 ||
+                TheLine->Last->TotalLength <= Style.ColumnLimit)
+                   ? 1
+                   : 0;
+      }
+      if (TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
+                                  tok::kw_for, TT_ForEachMacro)) {
+        return (Style.BraceWrapping.AfterControlStatement ==
+                FormatStyle::BWACS_Always)
+                   ? tryMergeSimpleBlock(I, E, Limit)
+                   : 0;
+      }
+      if (TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
+          Style.BraceWrapping.AfterControlStatement ==
+              FormatStyle::BWACS_MultiLine) {
+        // This case if different from the upper BWACS_MultiLine processing
+        // in that a preceding r_brace is not on the same line as else/catch
+        // most likely because of BeforeElse/BeforeCatch set to true.
+        // If the line length doesn't fit ColumnLimit, leave l_brace on the
+        // next line to respect the BWACS_MultiLine.
+        return (Style.ColumnLimit == 0 ||
+                TheLine->Last->TotalLength <= Style.ColumnLimit)
+                   ? 1
+                   : 0;
+      }
     }
-    // Don't merge block with left brace wrapped after ObjC special blocks
-    if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
-        I[-1]->First->is(tok::at) && I[-1]->First->Next) {
-      tok::ObjCKeywordKind kwId = I[-1]->First->Next->Tok.getObjCKeywordID();
-      if (kwId == clang::tok::objc_autoreleasepool ||
-          kwId == clang::tok::objc_synchronized)
+    if (PreviousLine && TheLine->First->is(tok::l_brace)) {
+      switch (PreviousLine->First->Tok.getKind()) {
+      case tok::at:
+        // Don't merge block with left brace wrapped after ObjC special blocks.
+        if (PreviousLine->First->Next) {
+          tok::ObjCKeywordKind kwId =
+              PreviousLine->First->Next->Tok.getObjCKeywordID();
+          if (kwId == tok::objc_autoreleasepool ||
+              kwId == tok::objc_synchronized)
+            return 0;
+        }
+        break;
+
+      case tok::kw_case:
+      case tok::kw_default:
+        // Don't merge block with left brace wrapped after case labels.
         return 0;
+
+      default:
+        break;
+      }
     }
-    // Don't merge block with left brace wrapped after case labels
-    if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
-        I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
-      return 0;
 
     // Don't merge an empty template class or struct if SplitEmptyRecords
     // is defined.
-    if (Style.BraceWrapping.SplitEmptyRecord &&
-        TheLine->Last->is(tok::l_brace) && I != AnnotatedLines.begin() &&
-        I[-1]->Last) {
-      const FormatToken *Previous = I[-1]->Last;
+    if (PreviousLine && Style.BraceWrapping.SplitEmptyRecord &&
+        TheLine->Last->is(tok::l_brace) && PreviousLine->Last) {
+      const FormatToken *Previous = PreviousLine->Last;
       if (Previous) {
         if (Previous->is(tok::comment))
           Previous = Previous->getPreviousNonComment();
         if (Previous) {
-          if (Previous->is(tok::greater) && !I[-1]->InPPDirective)
+          if (Previous->is(tok::greater) && !PreviousLine->InPPDirective)
             return 0;
           if (Previous->is(tok::identifier)) {
             const FormatToken *PreviousPrevious =
@@ -440,7 +448,7 @@ private:
       }
     }
 
-    // Try to merge a block with left brace unwrapped that wasn't yet covered
+    // Try to merge a block with left brace unwrapped that wasn't yet covered.
     if (TheLine->Last->is(tok::l_brace)) {
       const FormatToken *Tok = TheLine->First;
       bool ShouldMerge = false;
@@ -450,21 +458,21 @@ private:
       }
       if (Tok->isOneOf(tok::kw_class, tok::kw_struct)) {
         ShouldMerge = !Style.BraceWrapping.AfterClass ||
-                      (I[1]->First->is(tok::r_brace) &&
+                      (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyRecord);
       } else if (Tok->is(tok::kw_enum)) {
         ShouldMerge = Style.AllowShortEnumsOnASingleLine;
       } else {
         ShouldMerge = !Style.BraceWrapping.AfterFunction ||
-                      (I[1]->First->is(tok::r_brace) &&
+                      (NextLine.First->is(tok::r_brace) &&
                        !Style.BraceWrapping.SplitEmptyFunction);
       }
       return ShouldMerge ? tryMergeSimpleBlock(I, E, Limit) : 0;
     }
-    // Try to merge a function block with left brace wrapped
-    if (I[1]->First->is(TT_FunctionLBrace) &&
+    // Try to merge a function block with left brace wrapped.
+    if (NextLine.First->is(TT_FunctionLBrace) &&
         Style.BraceWrapping.AfterFunction) {
-      if (I[1]->Last->is(TT_LineComment))
+      if (NextLine.Last->is(TT_LineComment))
         return 0;
 
       // Check for Limit <= 2 to account for the " {".
@@ -475,7 +483,7 @@ private:
       unsigned MergedLines = 0;
       if (MergeShortFunctions ||
           (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
-           I[1]->First == I[1]->Last && I + 2 != E &&
+           NextLine.First == NextLine.Last && I + 2 != E &&
            I[2]->First->is(tok::r_brace))) {
         MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
         // If we managed to merge the block, count the function header, which is