Change indentation when breaking after a type.

clang-format did not indent any declarations/definitions when breaking
after the type. With this change, it indents for all declarations but
does not indent for function definitions, i.e.:

Before:
const SomeLongTypeName&
some_long_variable_name;
typedef SomeLongTypeName
SomeLongTypeAlias;
const SomeLongReturnType*
SomeLongFunctionName();
const SomeLongReturnType*
SomeLongFunctionName() { ... }

After:
const SomeLongTypeName&
    some_long_variable_name;
typedef SomeLongTypeName
    SomeLongTypeAlias;
const SomeLongReturnType*
    SomeLongFunctionName();
const SomeLongReturnType*
SomeLongFunctionName() { ... }

While it might seem inconsistent to indent function declarations, but
not definitions, there are two reasons for that:
- Function declarations are very similar to declarations of function
type variables, so there is another side to consistency to consider.
- There can be many function declarations on subsequent lines and not
indenting can make them harder to identify. Function definitions
are already separated by their body and not indenting
makes the function name slighly easier to find.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@181187 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp
index a76e0d48a49f742ae2a5b6ad1b609a965ce34207..d251d4f9e67e77d3a353dd7ad2d4231fd19d278f 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/lib/Format/Format.cpp
@@ -359,7 +359,8 @@ private:
                  State.Stack.back().VariablePos != 0) {
         State.Column = State.Stack.back().VariablePos;
       } else if (Previous.ClosesTemplateDeclaration ||
-                 (Current.Type == TT_StartOfName && State.ParenLevel == 0)) {
+                 (Current.Type == TT_StartOfName && State.ParenLevel == 0 &&
+                  Line.StartsDefinition)) {
         State.Column = State.Stack.back().Indent;
       } else if (Current.Type == TT_ObjCSelectorName) {
         if (State.Stack.back().ColonPos > Current.FormatTok.TokenLength) {

diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index fa7ef78451e61dfc8aeba44baa07bc2168c7c8f9..17abb01d181d5417659b3de917574c5448b64ab4 100644 (file)
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -245,24 +245,25 @@ private:
   }
 
   bool parseBrace() {
-    // Lines are fine to end with '{'.
-    if (CurrentToken == NULL)
-      return true;
-    ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
-    AnnotatedToken *Left = CurrentToken->Parent;
-    while (CurrentToken != NULL) {
-      if (CurrentToken->is(tok::r_brace)) {
-        Left->MatchingParen = CurrentToken;
-        CurrentToken->MatchingParen = Left;
-        next();
-        return true;
+    if (CurrentToken != NULL) {
+      ScopedContextCreator ContextCreator(*this, tok::l_brace, 1);
+      AnnotatedToken *Left = CurrentToken->Parent;
+      while (CurrentToken != NULL) {
+        if (CurrentToken->is(tok::r_brace)) {
+          Left->MatchingParen = CurrentToken;
+          CurrentToken->MatchingParen = Left;
+          next();
+          return true;
+        }
+        if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
+          return false;
+        updateParameterCount(Left, CurrentToken);
+        if (!consumeToken())
+          return false;
       }
-      if (CurrentToken->isOneOf(tok::r_paren, tok::r_square))
-        return false;
-      updateParameterCount(Left, CurrentToken);
-      if (!consumeToken())
-        return false;
     }
+    // No closing "}" found, this probably starts a definition.
+    Line.StartsDefinition = true;
     return true;
   }
 

diff --git a/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h
index fbb65b62549ba61d39ccde2ccb9c50ccbaac255c..b364082391f8e68e98c7aa29da75a6c038f1a754 100644 (file)
--- a/lib/Format/TokenAnnotator.h
+++ b/lib/Format/TokenAnnotator.h
@@ -209,8 +209,8 @@ public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front()), Level(Line.Level),
         InPPDirective(Line.InPPDirective),
-        MustBeDeclaration(Line.MustBeDeclaration),
-        MightBeFunctionDecl(false) {
+        MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
+        StartsDefinition(false) {
     assert(!Line.Tokens.empty());
     AnnotatedToken *Current = &First;
     for (std::list<FormatToken>::const_iterator I = ++Line.Tokens.begin(),
@@ -226,7 +226,8 @@ public:
       : First(Other.First), Type(Other.Type), Level(Other.Level),
         InPPDirective(Other.InPPDirective),
         MustBeDeclaration(Other.MustBeDeclaration),
-        MightBeFunctionDecl(Other.MightBeFunctionDecl) {
+        MightBeFunctionDecl(Other.MightBeFunctionDecl),
+        StartsDefinition(Other.StartsDefinition) {
     Last = &First;
     while (!Last->Children.empty()) {
       Last->Children[0].Parent = Last;
@@ -242,6 +243,7 @@ public:
   bool InPPDirective;
   bool MustBeDeclaration;
   bool MightBeFunctionDecl;
+  bool StartsDefinition;
 };
 
 inline prec::Level getPrecedence(const AnnotatedToken &Tok) {

diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 2b74506fc011388a46e2b8293e3f242c10183f89..98e6d47c4f440116654dd0683010ba02d58645af 100644 (file)
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -1520,7 +1520,7 @@ TEST_F(FormatTest, MixingPreprocessorDirectivesAndNormalCode) {
 TEST_F(FormatTest, LayoutStatementsAroundPreprocessorDirectives) {
   EXPECT_EQ("int\n"
             "#define A\n"
-            "a;",
+            "    a;",
             format("int\n#define A\na;"));
   verifyFormat("functionCallTo(\n"
                "    someOtherFunction(\n"
@@ -1713,7 +1713,7 @@ TEST_F(FormatTest, MemoizationTests) {
       "CFRunLoopTimerCreate(CFAllocatorRef allocato, CFAbsoluteTime fireDate,\n"
       "                     CFTimeInterval interval, CFOptionFlags flags,\n"
       "                     CFIndex order, CFRunLoopTimerCallBack callout,\n"
-      "                     CFRunLoopTimerContext *context);");
+      "                     CFRunLoopTimerContext *context) {}");
 
   // Deep nesting somewhat works around our memoization.
   verifyFormat(
@@ -1755,8 +1755,9 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) {
                "                              Cccccccccccccc cccccccccccccc);");
 
   // 2) break after return type.
-  verifyFormat("Aaaaaaaaaaaaaaaaaaaaaaaa\n"
-               "bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);");
+  verifyFormat(
+      "Aaaaaaaaaaaaaaaaaaaaaaaa\n"
+      "    bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);");
 
   // 3) break after (.
   verifyFormat(
@@ -1766,8 +1767,8 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) {
   // 4) break before after nested name specifiers.
   verifyFormat(
       "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "SomeClasssssssssssssssssssssssssssssssssssssss::\n"
-      "    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc);");
+      "    SomeClasssssssssssssssssssssssssssssssssssssss::\n"
+      "        bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc);");
 
   // However, there are exceptions, if a sufficient amount of lines can be
   // saved.
@@ -1780,9 +1781,9 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) {
                "                                  Cccccccccccccc cccccccccc);");
   verifyFormat(
       "Aaaaaaaaaaaaaaaaaa\n"
-      "bbbbbbbbbbbbbb(Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n"
-      "               Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n"
-      "               Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc);");
+      "    bbbbbbbbbbb(Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n"
+      "                Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n"
+      "                Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc);");
   verifyFormat(
       "Aaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc,\n"
       "                                          Cccccccccccccc cccccccccc,\n"
@@ -1954,12 +1955,9 @@ TEST_F(FormatTest, DoesNotBreakTrailingAnnotation) {
       "                   aaaaaaaaaaaaaaaaaaaaaaaaa));");
   verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "    __attribute__((unused));");
-
-  // FIXME: This is bad indentation, but generally hard to distinguish from a
-  // function declaration.
   verifyFormat(
       "bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-      "GUARDED_BY(aaaaaaaaaaaa);");
+      "    GUARDED_BY(aaaaaaaaaaaa);");
 }
 
 TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) {
@@ -2121,8 +2119,8 @@ TEST_F(FormatTest, DeclarationsOfMultipleVariables) {
   // FIXME: If multiple variables are defined, the "*" needs to move to the new
   // line. Also fix indent for breaking after the type, this looks bad.
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n"
-               "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaa,\n"
-               "    *b = bbbbbbbbbbbbbbbbbbb;");
+               "    aaaaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaa,\n"
+               "   *b = bbbbbbbbbbbbbbbbbbb;");
 
   // Not ideal, but pointer-with-type does not allow much here.
   verifyGoogleFormat(
@@ -2707,6 +2705,19 @@ TEST_F(FormatTest, FormatsFunctionTypes) {
 }
 
 TEST_F(FormatTest, BreaksLongDeclarations) {
+  verifyFormat("typedef LoooooooooooooooooooooooooooooooooooooooongType\n"
+               "    AnotherNameForTheLongType;");
+  verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n"
+               "    LoooooooooooooooooooooooooooooooooooooooongVariable;");
+  verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType\n"
+               "    LoooooooooooooooooooooooooooooooongFunctionDeclaration();");
+  verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType\n"
+               "LooooooooooooooooooooooooooooooooooongFunctionDefinition() {}");
+
+  // FIXME: Without the comment, this breaks after "(".
+  verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType // break\n"
+               "    (*LoooooooooooooooooooooooooooongFunctionTypeVarialbe)();");
+
   verifyFormat("int *someFunction(int LoooooooooooooooooooongParam1,\n"
                "                  int LoooooooooooooooooooongParam2) {}");
   verifyFormat(
@@ -2722,7 +2733,7 @@ TEST_F(FormatTest, BreaksLongDeclarations) {
                "        AnotherLongParameterName) {}");
   verifyFormat(
       "aaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaa<aaaaaaaaaaaaa, aaaaaaaaaaaa>\n"
-      "aaaaaaaaaaaaaaaaaaaaaaa;");
+      "    aaaaaaaaaaaaaaaaaaaaaaa;");
 
   verifyGoogleFormat(
       "TypeSpecDecl* TypeSpecDecl::Create(ASTContext& C, DeclContext* DC,\n"