From: Alexander Kornienko Date: Wed, 20 Nov 2013 16:33:05 +0000 (+0000) Subject: Added an option to allow short function bodies be placed on a single line. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=29997cd9a54aad6400ba7c97c4a2be3d9f4a2e8b;p=clang Added an option to allow short function bodies be placed on a single line. Summary: The AllowShortFunctionsOnASingleLine option now controls short function body placement on a single line independent of the BreakBeforeBraces option. Updated tests using BreakBeforeBraces other than BS_Attach. Addresses http://llvm.org/PR17888 Reviewers: klimek, djasper Reviewed By: klimek CC: cfe-commits, klimek Differential Revision: http://llvm-reviews.chandlerc.com/D2230 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@195256 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h index 0f2746712a..40259705dd 100644 --- a/include/clang/Format/Format.h +++ b/include/clang/Format/Format.h @@ -141,6 +141,10 @@ struct FormatStyle { /// single line. bool AllowShortLoopsOnASingleLine; + /// \brief If \c true, int f() { return 0; } can be put on a single + /// line. + bool AllowShortFunctionsOnASingleLine; + /// \brief Add a space in front of an Objective-C protocol list, i.e. use /// Foo instead of \c Foo. bool ObjCSpaceBeforeProtocolList; @@ -255,6 +259,8 @@ struct FormatStyle { AlignTrailingComments == R.AlignTrailingComments && AllowAllParametersOfDeclarationOnNextLine == R.AllowAllParametersOfDeclarationOnNextLine && + AllowShortFunctionsOnASingleLine == + R.AllowShortFunctionsOnASingleLine && AllowShortIfStatementsOnASingleLine == R.AllowShortIfStatementsOnASingleLine && AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine && diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 01c122ecc7..58fe385a03 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -117,6 +117,8 @@ template <> struct MappingTraits { Style.AllowShortIfStatementsOnASingleLine); IO.mapOptional("AllowShortLoopsOnASingleLine", Style.AllowShortLoopsOnASingleLine); + IO.mapOptional("AllowShortFunctionsOnASingleLine", + Style.AllowShortFunctionsOnASingleLine); IO.mapOptional("AlwaysBreakTemplateDeclarations", Style.AlwaysBreakTemplateDeclarations); IO.mapOptional("AlwaysBreakBeforeMultilineStrings", @@ -190,6 +192,7 @@ FormatStyle getLLVMStyle() { LLVMStyle.AlignEscapedNewlinesLeft = false; LLVMStyle.AlignTrailingComments = true; LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true; + LLVMStyle.AllowShortFunctionsOnASingleLine = true; LLVMStyle.AllowShortIfStatementsOnASingleLine = false; LLVMStyle.AllowShortLoopsOnASingleLine = false; LLVMStyle.AlwaysBreakBeforeMultilineStrings = false; @@ -237,6 +240,7 @@ FormatStyle getGoogleStyle() { GoogleStyle.AlignEscapedNewlinesLeft = true; GoogleStyle.AlignTrailingComments = true; GoogleStyle.AllowAllParametersOfDeclarationOnNextLine = true; + GoogleStyle.AllowShortFunctionsOnASingleLine = true; GoogleStyle.AllowShortIfStatementsOnASingleLine = true; GoogleStyle.AllowShortLoopsOnASingleLine = true; GoogleStyle.AlwaysBreakBeforeMultilineStrings = true; @@ -381,7 +385,7 @@ public: /// \brief Calculates how many lines can be merged into 1 starting at \p I. unsigned tryFitMultipleLinesInOne(unsigned Indent, - SmallVectorImpl::const_iterator &I, + SmallVectorImpl::const_iterator I, SmallVectorImpl::const_iterator E) { // We can never merge stuff if there are trailing line comments. AnnotatedLine *TheLine = *I; @@ -402,16 +406,43 @@ public: if (I + 1 == E || I[1]->Type == LT_Invalid) return 0; + if (TheLine->Last->Type == TT_FunctionLBrace) { + return Style.AllowShortFunctionsOnASingleLine + ? tryMergeSimpleBlock(I, E, Limit) + : 0; + } if (TheLine->Last->is(tok::l_brace)) { - return tryMergeSimpleBlock(I, E, Limit); - } else if (Style.AllowShortIfStatementsOnASingleLine && - TheLine->First->is(tok::kw_if)) { - return tryMergeSimpleControlStatement(I, E, Limit); - } else if (Style.AllowShortLoopsOnASingleLine && - TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { - return tryMergeSimpleControlStatement(I, E, Limit); - } else if (TheLine->InPPDirective && (TheLine->First->HasUnescapedNewline || - TheLine->First->IsFirst)) { + return Style.BreakBeforeBraces == clang::format::FormatStyle::BS_Attach + ? tryMergeSimpleBlock(I, E, Limit) + : 0; + } + if (I[1]->First->Type == TT_FunctionLBrace && + Style.BreakBeforeBraces != FormatStyle::BS_Attach) { + // Reduce the column limit by the number of spaces we need to insert + // around braces. + Limit = Limit > 3 ? Limit - 3 : 0; + unsigned MergedLines = 0; + if (Style.AllowShortFunctionsOnASingleLine) { + MergedLines = tryMergeSimpleBlock(I + 1, E, Limit); + // If we managed to merge the block, count the function header, which is + // on a separate line. + if (MergedLines > 0) + ++MergedLines; + } + return MergedLines; + } + if (TheLine->First->is(tok::kw_if)) { + return Style.AllowShortIfStatementsOnASingleLine + ? tryMergeSimpleControlStatement(I, E, Limit) + : 0; + } + if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { + return Style.AllowShortLoopsOnASingleLine + ? tryMergeSimpleControlStatement(I, E, Limit) + : 0; + } + if (TheLine->InPPDirective && + (TheLine->First->HasUnescapedNewline || TheLine->First->IsFirst)) { return tryMergeSimplePPDirective(I, E, Limit); } return 0; @@ -419,7 +450,7 @@ public: private: unsigned - tryMergeSimplePPDirective(SmallVectorImpl::const_iterator &I, + tryMergeSimplePPDirective(SmallVectorImpl::const_iterator I, SmallVectorImpl::const_iterator E, unsigned Limit) { if (Limit == 0) @@ -434,7 +465,7 @@ private: } unsigned tryMergeSimpleControlStatement( - SmallVectorImpl::const_iterator &I, + SmallVectorImpl::const_iterator I, SmallVectorImpl::const_iterator E, unsigned Limit) { if (Limit == 0) return 0; @@ -450,7 +481,7 @@ private: if (1 + I[1]->Last->TotalLength > Limit) return 0; if (I[1]->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for, - tok::kw_while) || + tok::kw_while) || I[1]->First->Type == TT_LineComment) return 0; // Only inline simple if's (no nested if or else). @@ -461,13 +492,9 @@ private: } unsigned - tryMergeSimpleBlock(SmallVectorImpl::const_iterator &I, + tryMergeSimpleBlock(SmallVectorImpl::const_iterator I, SmallVectorImpl::const_iterator E, unsigned Limit) { - // No merging if the brace already is on the next line. - if (Style.BreakBeforeBraces != FormatStyle::BS_Attach) - return 0; - // First, check that the current line allows merging. This is the case if // we're not in a control flow statement and the last token is an opening // brace. @@ -583,8 +610,7 @@ public: } } else if (TheLine.Type != LT_Invalid && (WasMoved || FormatPPDirective || touchesLine(TheLine))) { - unsigned LevelIndent = - getIndent(IndentForLevel, TheLine.Level); + unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level); if (FirstTok->WhitespaceRange.isValid()) { if (!DryRun) formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level, @@ -732,9 +758,9 @@ private: if (PreviousLine && PreviousLine->First->isAccessSpecifier()) Newlines = std::min(1u, Newlines); - Whitespaces->replaceWhitespace( - RootToken, Newlines, IndentLevel, Indent, Indent, - InPPDirective && !RootToken.HasUnescapedNewline); + Whitespaces->replaceWhitespace(RootToken, Newlines, IndentLevel, Indent, + Indent, InPPDirective && + !RootToken.HasUnescapedNewline); } /// \brief Get the indent of \p Level from \p IndentForLevel. @@ -961,7 +987,7 @@ private: // Cannot merge multiple statements into a single line. if (Previous.Children.size() > 1) - return false; + return false; // We can't put the closing "}" on a line with a trailing comment. if (Previous.Children[0]->Last->isTrailingComment()) diff --git a/lib/Format/FormatToken.h b/lib/Format/FormatToken.h index 2145ee28dc..5ac686183b 100644 --- a/lib/Format/FormatToken.h +++ b/lib/Format/FormatToken.h @@ -39,6 +39,7 @@ enum TokenType { TT_ImplicitStringLiteral, TT_InlineASMColon, TT_InheritanceColon, + TT_FunctionLBrace, TT_FunctionTypeLParen, TT_LambdaLSquare, TT_LineComment, diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 3aee3a0394..fe5ab5df03 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/lib/Format/TokenAnnotator.cpp @@ -538,6 +538,7 @@ private: // Reset token type in case we have already looked at it and then // recovered from an error (e.g. failure to find the matching >). if (CurrentToken->Type != TT_LambdaLSquare && + CurrentToken->Type != TT_FunctionLBrace && CurrentToken->Type != TT_ImplicitStringLiteral) CurrentToken->Type = TT_Unknown; if (CurrentToken->Role) diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp index e0b090f6ab..d074308101 100644 --- a/lib/Format/UnwrappedLineParser.cpp +++ b/lib/Format/UnwrappedLineParser.cpp @@ -681,6 +681,7 @@ void UnwrappedLineParser::parseStructuralElement() { Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup || Style.BreakBeforeBraces == FormatStyle::BS_Allman) addUnwrappedLine(); + FormatTok->Type = TT_FunctionLBrace; parseBlock(/*MustBeDeclaration=*/false); addUnwrappedLine(); return; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 433f3f70c8..0d317e65b6 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -4776,7 +4776,14 @@ TEST_F(FormatTest, FormatsBracedListsInColumnLayout) { } TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) { + FormatStyle DoNotMerge = getLLVMStyle(); + DoNotMerge.AllowShortFunctionsOnASingleLine = false; + verifyFormat("void f() { return 42; }"); + verifyFormat("void f() {\n" + " return 42;\n" + "}", + DoNotMerge); verifyFormat("void f() {\n" " // Comment\n" "}"); @@ -4790,6 +4797,13 @@ TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) { "}"); verifyFormat("void f() {} // comment"); verifyFormat("void f() { int a; } // comment"); + verifyFormat("void f() {\n" + "} // comment", + DoNotMerge); + verifyFormat("void f() {\n" + " int a;\n" + "} // comment", + DoNotMerge); verifyFormat("void f() {\n" "} // comment", getLLVMStyleWithColumns(15)); @@ -6613,10 +6627,7 @@ TEST_F(FormatTest, LinuxBraceBreaking) { " b();\n" " }\n" " }\n" - " void g()\n" - " {\n" - " return;\n" - " }\n" + " void g() { return; }\n" "}\n" "}", BreakBeforeBrace); @@ -6634,10 +6645,7 @@ TEST_F(FormatTest, StroustrupBraceBreaking) { " b();\n" " }\n" " }\n" - " void g()\n" - " {\n" - " return;\n" - " }\n" + " void g() { return; }\n" "}\n" "}", BreakBeforeBrace); @@ -6658,10 +6666,7 @@ TEST_F(FormatTest, AllmanBraceBreaking) { " b();\n" " }\n" " }\n" - " void g()\n" - " {\n" - " return;\n" - " }\n" + " void g() { return; }\n" "}\n" "}", BreakBeforeBrace); @@ -6822,6 +6827,7 @@ TEST_F(FormatTest, ParsesConfiguration) { CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft); CHECK_PARSE_BOOL(AlignTrailingComments); CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine); + CHECK_PARSE_BOOL(AllowShortFunctionsOnASingleLine); CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine); CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine); CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations); @@ -7126,7 +7132,7 @@ TEST_F(FormatTest, FormatsWithWebKitStyle) { " : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" " , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n" " aaaaaaaaaaaaaa)\n" - " , aaaaaaaaaaaaaaaaaaaaaaa()\n{\n}", + " , aaaaaaaaaaaaaaaaaaaaaaa() {}", Style); // Access specifiers should be aligned left.