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

llvm-svn: 195256

clang/include/clang/Format/Format.h

@@ -141,6 +141,10 @@ struct FormatStyle {
   /// single line.
   bool AllowShortLoopsOnASingleLine;
 
+  /// \brief If \c true, <tt>int f() { return 0; }</tt> can be put on a single
+  /// line.
+  bool AllowShortFunctionsOnASingleLine;
+
   /// \brief Add a space in front of an Objective-C protocol list, i.e. use
   /// <tt>Foo <Protocol></tt> instead of \c Foo<Protocol>.
   bool ObjCSpaceBeforeProtocolList;
@@ -255,6 +259,8 @@ struct FormatStyle {
            AlignTrailingComments == R.AlignTrailingComments &&
            AllowAllParametersOfDeclarationOnNextLine ==
                R.AllowAllParametersOfDeclarationOnNextLine &&
+           AllowShortFunctionsOnASingleLine ==
+               R.AllowShortFunctionsOnASingleLine &&
            AllowShortIfStatementsOnASingleLine ==
                R.AllowShortIfStatementsOnASingleLine &&
            AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&

clang/lib/Format/Format.cpp

@@ -117,6 +117,8 @@ template <> struct MappingTraits<clang::format::FormatStyle> {
                    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<AnnotatedLine *>::const_iterator &I,
+                           SmallVectorImpl<AnnotatedLine *>::const_iterator I,
                            SmallVectorImpl<AnnotatedLine *>::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);
+      return Style.BreakBeforeBraces == clang::format::FormatStyle::BS_Attach
-    } else if (Style.AllowShortIfStatementsOnASingleLine &&
+                 ? tryMergeSimpleBlock(I, E, Limit)
-               TheLine->First->is(tok::kw_if)) {
+                 : 0;
-      return tryMergeSimpleControlStatement(I, E, Limit);
+    }
-    } else if (Style.AllowShortLoopsOnASingleLine &&
+    if (I[1]->First->Type == TT_FunctionLBrace &&
-               TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+        Style.BreakBeforeBraces != FormatStyle::BS_Attach) {
-      return tryMergeSimpleControlStatement(I, E, Limit);
+      // Reduce the column limit by the number of spaces we need to insert
-    } else if (TheLine->InPPDirective && (TheLine->First->HasUnescapedNewline ||
+      // around braces.
-                                          TheLine->First->IsFirst)) {
+      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<AnnotatedLine *>::const_iterator &I,
+  tryMergeSimplePPDirective(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
                             SmallVectorImpl<AnnotatedLine *>::const_iterator E,
                             unsigned Limit) {
     if (Limit == 0)
@@ -434,7 +465,7 @@ private:
   }
 
   unsigned tryMergeSimpleControlStatement(
-      SmallVectorImpl<AnnotatedLine *>::const_iterator &I,
+      SmallVectorImpl<AnnotatedLine *>::const_iterator I,
       SmallVectorImpl<AnnotatedLine *>::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<AnnotatedLine *>::const_iterator &I,
+  tryMergeSimpleBlock(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
                       SmallVectorImpl<AnnotatedLine *>::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 =
+        unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level);
-            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(
+    Whitespaces->replaceWhitespace(RootToken, Newlines, IndentLevel, Indent,
-        RootToken, Newlines, IndentLevel, Indent, Indent,
+                                   Indent, InPPDirective &&
-        InPPDirective && !RootToken.HasUnescapedNewline);
+                                               !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())

clang/lib/Format/FormatToken.h

@@ -39,6 +39,7 @@ enum TokenType {
   TT_ImplicitStringLiteral,
   TT_InlineASMColon,
   TT_InheritanceColon,
+  TT_FunctionLBrace,
   TT_FunctionTypeLParen,
   TT_LambdaLSquare,
   TT_LineComment,

clang/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)

clang/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;

clang/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"
+               "  void g() { return; }\n"
-               "  {\n"
-               "    return;\n"
-               "  }\n"
                "}\n"
                "}",
                BreakBeforeBrace);
@@ -6634,10 +6645,7 @@ TEST_F(FormatTest, StroustrupBraceBreaking) {
                "      b();\n"
                "    }\n"
                "  }\n"
-               "  void g()\n"
+               "  void g() { return; }\n"
-               "  {\n"
-               "    return;\n"
-               "  }\n"
                "}\n"
                "}",
                BreakBeforeBrace);
@@ -6658,10 +6666,7 @@ TEST_F(FormatTest, AllmanBraceBreaking) {
                "      b();\n"
                "    }\n"
                "  }\n"
-               "  void g()\n"
+               "  void g() { return; }\n"
-               "  {\n"
-               "    return;\n"
-               "  }\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.