Author: Björn Schäpers Date: 2022-02-03T22:55:27+01:00 New Revision: 3d0b619261392f5f24a7b9961b24cdd621829c13
URL: https://github.com/llvm/llvm-project/commit/3d0b619261392f5f24a7b9961b24cdd621829c13 DIFF: https://github.com/llvm/llvm-project/commit/3d0b619261392f5f24a7b9961b24cdd621829c13.diff LOG: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter * 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 Added: Modified: clang/lib/Format/UnwrappedLineFormatter.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index f7d26b813f4f..9f49410f741a 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -228,10 +228,11 @@ class LineJoiner { 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 @@ class LineJoiner { 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 @@ class LineJoiner { 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 @@ class LineJoiner { 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 @@ class LineJoiner { 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 @@ class LineJoiner { ? 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 diff erent 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 diff erent 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 @@ class LineJoiner { } } - // 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 @@ class LineJoiner { } 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 @@ class LineJoiner { 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits