[PATCH] D42729: clang-format: Fix formatting of function blocks followed by semicolon

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

In some case, the heuristic used to identify the type of blocks (in
UnwrappedLineParser) mistakens a Block for a BracedInit, when the
TokenAnnotator eventually finds (correclty) that this is a function
declaration.

This happens with empty function blocks followed by a semicolon.

This causes for exemple the following code to be mistakenly formatted:

  void abort(){};

instead of:

  void abort() {};


Repository:
  rC Clang

https://reviews.llvm.org/D42729

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11386,6 +11386,15 @@
                "};");
 }
 
+TEST_F(FormatTest, MunchSemicolonAfterFunction) {
+  verifyFormat("void foo() { int i; };");
+  verifyFormat("void foo() {\n"
+               "  int i;\n"
+               "  int j;\n"
+               "};");
+  verifyFormat("void foo() {};");
+}
+
 TEST_F(FormatTest, ConfigurableContinuationIndentWidth) {
   FormatStyle TwoIndent = getLLVMStyleWithColumns(15);
   TwoIndent.ContinuationIndentWidth = 2;
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1783,9 +1783,12 @@
 }
 
 // This function heuristically determines whether 'Current' starts the name of a
-// function declaration.
-static bool isFunctionDeclarationName(const FormatToken &Current,
-                                      const AnnotatedLine &Line) {
+// function declaration, and returns the first token of parameters list in that
+// case.
+// @return The token which opens the parameters list (e.g. the opening
+// parenthesis), or nullptr if this is not a function declaration.
+static const FormatToken *getFunctionParameterList(const FormatToken &Current,
+                                                   const AnnotatedLine &Line) {
   auto skipOperatorName = [](const FormatToken *Next) -> const FormatToken * {
     for (; Next; Next = Next->Next) {
       if (Next->is(TT_OverloadedOperatorLParen))
@@ -1809,58 +1812,58 @@
   const FormatToken *Next = Current.Next;
   if (Current.is(tok::kw_operator)) {
     if (Current.Previous && Current.Previous->is(tok::coloncolon))
-      return false;
+      return nullptr;
     Next = skipOperatorName(Next);
   } else {
     if (!Current.is(TT_StartOfName) || Current.NestingLevel != 0)
-      return false;
+      return nullptr;
     for (; Next; Next = Next->Next) {
       if (Next->is(TT_TemplateOpener)) {
         Next = Next->MatchingParen;
       } else if (Next->is(tok::coloncolon)) {
         Next = Next->Next;
         if (!Next)
-          return false;
+          return nullptr;
         if (Next->is(tok::kw_operator)) {
           Next = skipOperatorName(Next->Next);
           break;
         }
         if (!Next->is(tok::identifier))
-          return false;
+          return nullptr;
       } else if (Next->is(tok::l_paren)) {
         break;
       } else {
-        return false;
+        return nullptr;
       }
     }
   }
 
   // Check whether parameter list can belong to a function declaration.
   if (!Next || !Next->is(tok::l_paren) || !Next->MatchingParen)
-    return false;
+    return nullptr;
   // If the lines ends with "{", this is likely an function definition.
   if (Line.Last->is(tok::l_brace))
-    return true;
+    return Next;
   if (Next->Next == Next->MatchingParen)
-    return true; // Empty parentheses.
+    return Next; // Empty parentheses.
   // If there is an &/&& after the r_paren, this is likely a function.
   if (Next->MatchingParen->Next &&
       Next->MatchingParen->Next->is(TT_PointerOrReference))
-    return true;
+    return Next;
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
        Tok = Tok->Next) {
     if (Tok->is(tok::l_paren) && Tok->MatchingParen) {
       Tok = Tok->MatchingParen;
       continue;
     }
     if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||
         Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis))
-      return true;
+      return Next;
     if (Tok->isOneOf(tok::l_brace, tok::string_literal, TT_ObjCMethodExpr) ||
         Tok->Tok.isLiteral())
-      return false;
+      return nullptr;
   }
-  return false;
+  return nullptr;
 }
 
 bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line) const {
@@ -1899,8 +1902,14 @@
   FormatToken *Current = Line.First->Next;
   bool InFunctionDecl = Line.MightBeFunctionDecl;
   while (Current) {
-    if (isFunctionDeclarationName(*Current, Line))
+    if (const FormatToken *Tok = getFunctionParameterList(*Current, Line)) {
       Current->Type = TT_FunctionDeclarationName;
+
+      // Fix block kind, if it was mistakenly identified as a BracedInit
+      FormatToken *Next = Tok->MatchingParen->Next;
+      if (Next && Next->is(tok::l_brace) && Next->BlockKind == BK_BracedInit)
+          Next->BlockKind = BK_Block;
+    }
     if (Current->is(TT_LineComment)) {
       if (Current->Previous->BlockKind == BK_BracedInit &&
           Current->Previous->opensScope())

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits