Leave alone the semicolon after the MacroBlockBegin name

[owenpan]

Summary

This patch fixes the bug below.

The code:

FOO_BEGIN();
  FOO_ENTRY
FOO_END();

is erroneously formatted to:

FOO_BEGIN()
  ;
  FOO_ENTRY
FOO_END();

[sammccall]

The documentation examples of MacroBlockBegin are:

/// A regular expression matching macros that start a block.
/// \code
///    # With:
///    MacroBlockBegin: "^NS_MAP_BEGIN|\
///    NS_TABLE_HEAD$"
///    MacroBlockEnd: "^\
///    NS_MAP_END|\
///    NS_TABLE_.*_END$"
///
///    NS_MAP_BEGIN
///      foo();
///    NS_MAP_END
///
///    NS_TABLE_HEAD
///      bar();
///    NS_TABLE_FOO_END
///
///    # Without:
///    NS_MAP_BEGIN
///    foo();
///    NS_MAP_END
///
///    NS_TABLE_HEAD
///    bar();
///    NS_TABLE_FOO_END
/// \endcode

It doesn't appear that a semicolon after the macro is expected, so the semicolon is treated as an empty statement inside the block.

There are some reasonable arguments for this. semicolons go after statement-like things, block introducers are not statement-like in general. Even if the particular implementation requires a semicolon (e.g. glBegin()), it breaks abstraction to make the user type it, it should be part of the macro instead.

If this doesn't work for your codebase, can you share some details about the macro (what it's for and what it expands to) that motivates this change?

[owenpan]

The semicolon is redundant but makes the MacroBlockBegin name stand out more. Nonetheless, I agree it's not a bug and we should leave the current behavior as is.

clang/lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
   size_t PPStartHash = computePPHash();
 
   unsigned InitialLevel = Line->Level;
   nextToken(/*LevelDifference=*/AddLevel ? 1 : 0);
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
     parseParens();
 
+  if (MunchSemi && FormatTok->Tok.is(tok::semi))
+    nextToken();
+
   size_t NbPreprocessorDirectives =
       CurrentLines == &Lines ? PreprocessorDirectives.size() : 0;
   addUnwrappedLine();
   size_t OpeningLineIndex =
       CurrentLines->empty()
           ? (UnwrappedLine::kInvalidIndex)
           : (CurrentLines->size() - 1 - NbPreprocessorDirectives);
 

clang/unittests/Format/FormatTest.cpp

verifyFormat("FOO_BEGIN\n"
                "  NESTED_FOO_BEGIN\n"
                "    NESTED_FOO_ENTRY\n"
                "  NESTED_FOO_END\n"
                "FOO_END", Style);
   verifyFormat("FOO_BEGIN(Foo, Bar)\n"
                "  int x;\n"
                "  x = 1;\n"
                "FOO_END(Baz)", Style);
+  verifyFormat("FOO_BEGIN();\n"
+               "  FOO_ENTRY\n"
+               "FOO_END();", Style);
 }
 
 //===----------------------------------------------------------------------===//
 // Line break tests.
 //===----------------------------------------------------------------------===//
 
 TEST_F(FormatTest, PreventConfusingIndents) {
   verifyFormat(