clang-format: use AfterControlStatement to format ObjC control blocks

[Typz]

Summary

ObjC defines @autoreleasepool and @synchronized control blocks. These
used to be formatted according to the AfterObjCDeclaration brace-
wrapping flag, which is not very consistent.

This patch changes the behavior to use the AfterControlStatement flag
instead. This should not affect the behavior unless a custom brace
wrapping mode is used.

[Typz]

This is done as discussed in https://reviews.llvm.org/D43114.
But I can instead add a new AfterObjCSpecialBlock brace wrapping flag.

[benhamilton]

Thanks! Can you add a test for this, please?

[Typz]

In D43232#1008004, @benhamilton wrote:

Thanks! Can you add a test for this, please?

There are actually tests, though they are performed by activating Allman brace wrapping mode.
I am trying to change them, but this highlights another issue: when only AfterControlStatement is set, the block gets inlined, like this:

@autoreleasepool
{ f(); }

This used to happen before as well, and would require fixing UnwrappedLineFormatter.cpp to understand that these blocks are linked to the previous ObjC keyword.

[benhamilton]

This used to happen before as well, and would require fixing UnwrappedLineFormatter.cpp to understand that these blocks are linked to the previous ObjC keyword.

Good find! Looks like we should fix that and add some more tests with AfterControlStatement set to true and to false.

[Typz]

I'll try to fix this.

Can you please review D43114 in the mean time? This patch depends on it, so it must be merged first.

[Typz]

Fix bug which was lingering: prevent inlining of ObjC special control blocks.

cfe/trunk/include/clang/Format/Format.h

struct BraceWrappingFlags {
     ///
     ///   false:
     ///   namespace {
     ///   int foo();
     ///   int bar();
     ///   }
     /// \endcode
     bool AfterNamespace;
-    /// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
+    /// \brief Wrap ObjC definitions (interfaces, implementations...).
+    /// \note @autoreleasepool and @synchronized blocks are wrapped
+    /// according to `AfterControlStatement` flag.
     bool AfterObjCDeclaration;
     /// \brief Wrap struct definitions.
     /// \code
     ///   true:
     ///   struct foo
     ///   {
     ///     int x;
     ///   };

cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp

if (TheLine->First->is(tok::l_brace) && TheLine->First == TheLine->Last &&
         MergedLines = tryMergeSimpleBlock(I - 1, E, Limit);
         // If we managed to merge the block, discard the first merged line
         // since we are merging starting from I.
         if (MergedLines > 0)
           --MergedLines;
       }
       return MergedLines;
     }
+    // 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)
+        return 0;
+    }
     // Try to merge a block with left brace wrapped that wasn't yet covered
     if (TheLine->Last->is(tok::l_brace)) {
       return !Style.BraceWrapping.AfterFunction ||
                      (I[1]->First->is(tok::r_brace) &&
                       !Style.BraceWrapping.SplitEmptyRecord)
                  ? tryMergeSimpleBlock(I, E, Limit)
                  : 0;
     }

cfe/trunk/lib/Format/UnwrappedLineParser.cpp

case tok::at:
       case tok::objc_optional:
       case tok::objc_required:
         nextToken();
         addUnwrappedLine();
         return;
       case tok::objc_autoreleasepool:
         nextToken();
         if (FormatTok->Tok.is(tok::l_brace)) {
-          if (Style.BraceWrapping.AfterObjCDeclaration)
+          if (Style.BraceWrapping.AfterControlStatement)
             addUnwrappedLine();
           parseBlock(/*MustBeDeclaration=*/false);
         }
         addUnwrappedLine();
         return;
       case tok::objc_synchronized:
         nextToken();
         if (FormatTok->Tok.is(tok::l_paren))
            // Skip synchronization object
            parseParens();
         if (FormatTok->Tok.is(tok::l_brace)) {
-          if (Style.BraceWrapping.AfterObjCDeclaration)
+          if (Style.BraceWrapping.AfterControlStatement)
             addUnwrappedLine();
           parseBlock(/*MustBeDeclaration=*/false);
         }
         addUnwrappedLine();
         return;
       case tok::objc_try:
         // This branch isn't strictly necessary (the kw_try case below would
         // do this too after the tok::at is parsed above).  But be explicit.

cfe/trunk/unittests/Format/FormatTestObjC.cpp

 
 TEST_F(FormatTestObjC, FormatObjCAutoreleasepool) {
   verifyFormat("@autoreleasepool {\n"
                "  f();\n"
                "}\n"
                "@autoreleasepool {\n"
                "  f();\n"
                "}\n");
-  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
   verifyFormat("@autoreleasepool\n"
                "{\n"
                "  f();\n"
                "}\n"
                "@autoreleasepool\n"
                "{\n"
                "  f();\n"
                "}\n");

 
 TEST_F(FormatTestObjC, FormatObjCSynchronized) {
   verifyFormat("@synchronized(self) {\n"
                "  f();\n"
                "}\n"
                "@synchronized(self) {\n"
                "  f();\n"
                "}\n");
-  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
   verifyFormat("@synchronized(self)\n"
                "{\n"
                "  f();\n"
                "}\n"
                "@synchronized(self)\n"
                "{\n"
                "  f();\n"
                "}\n");