clang-format: Add Block{Begin,End}Macros option

[poiru]

Summary

This adds the BlockBeginMacros and BlockEndMacros options that can be
used to make certain macros add indent levels similarly to '{' and '}',
respectively.

Mozilla code, for example, uses several macros that begin and end a scope.
Previously, Clang-Format removed the indentation resulting in:

MACRO_BEGIN(...)
MACRO_ENTRY(...)
MACRO_ENTRY(...)
MACRO_END

Now, using e.g. 'BlockBeginMacros: "^[A-Z_]+_BEGIN$"' and
'BlockEndMacros: "^[A-Z_]+_END$"', the result is as expected:

MACRO_BEGIN(...)

MACRO_ENTRY(...)
MACRO_ENTRY(...)

MACRO_END

[klimek]

Heya, thanks for working on this. I think we'll want to make this a bit more generic - I'm not saying the first version needs all features, but we should think about how we want to put it into the config with the future in mind.

Specifically, we'll want to specify macros that:

• open blocks
• open declaration contexts (class) (MS' test framework uses those)
• are full statements (currently we have heuristics for that, but those are limited)

[djasper]

include/clang/Format/Format.h

159 ↗

(On Diff #28788)

I think this should be consistent with 'ForEachMacros' and whatever other macro kinds we are going to introduce (see Manuel's comment). Specifically, all of them should either be a regular expression or a list of strings. I am fine with turning them into regular expressions.

I wonder whether all of these would be more discoverable if we chose the name so that they are consecutive, e.g. MacrosBlockBegin, MacrosBlockEnd, MacrosForEach. But I am not sure. Any thoughts?

lib/Format/Format.cpp

460 ↗

(On Diff #28788)

Can you set values for the new flags here, so it properly handles IPC_BEGIN_MESSAGE_MAP and IPC_END_MESSAGE_MAP?

477 ↗

(On Diff #28788)

Do you have good defaults here?

1168–1180 ↗

(On Diff #28788)

Can you wrap this entire if statement in an:

if (Style.Language == FormatStyle::LK_Cpp) {
}

?

This isn't relevant to other languages and matching every single identifier against a regular expression seems like a waste then.

lib/Format/UnwrappedLineParser.cpp

404 ↗

(On Diff #28788)

I'd (very slightly) prefer to keep this assert at the top, i.e.:

assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) && "...");
const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);

421–422 ↗

(On Diff #28788)

Can you pull the "!" into the ternary expression? Otherwise this is just hard to read for me (can't really explain why).

[poiru]

include/clang/Format/Format.h

159 ↗

(On Diff #28788)

I went with MacroBlock{Begin,End} because the plural at the beginning sounds weird. I'll add a regex-based MacroForEach in a future patch.

lib/Format/Format.cpp

477 ↗

(On Diff #28788)

Yes, but I think we (Mozilla) would prefer to keep them in our .clang-format file because the set of macros is likely to change.

[poiru]

Review comments addressed.

[djasper]

Thanks. Very nice :-).

[thakis]

I noticed that using this slows down clang-format about 300% -- https://llvm.org/bugs/show_bug.cgi?id=30656

cfe/trunk/docs/ClangFormatStyleOptions.rst

* ``LK_Java`` (in configuration: ``Java``)
     Should be used for Java.
   * ``LK_JavaScript`` (in configuration: ``JavaScript``)
     Should be used for JavaScript.
   * ``LK_Proto`` (in configuration: ``Proto``)
     Should be used for Protocol Buffers
     (https://developers.google.com/protocol-buffers/).
 
 
+**MacroBlockBegin** (``std::string``)
+  A regular expression matching macros that start a block.
+
+**MacroBlockEnd** (``std::string``)
+  A regular expression matching macros that end a block.
+
 **MaxEmptyLinesToKeep** (``unsigned``)
   The maximum number of consecutive empty lines to keep.
 
 **NamespaceIndentation** (``NamespaceIndentationKind``)
   The indentation used for namespaces.
 
   Possible values:
 

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

enum LanguageKind {
     /// Should be used for Protocol Buffers
     /// (https://developers.google.com/protocol-buffers/).
     LK_Proto
   };
 
   /// \brief Language, this format style is targeted at.
   LanguageKind Language;
 
+  /// \brief A regular expression matching macros that start a block.
+  std::string MacroBlockBegin;
+
+  /// \brief A regular expression matching macros that end a block.
+  std::string MacroBlockEnd;
+
   /// \brief The maximum number of consecutive empty lines to keep.
   unsigned MaxEmptyLinesToKeep;
 
   /// \brief Different ways to indent namespace contents.
   enum NamespaceIndentationKind {
     /// Don't indent in namespaces.
     NI_None,
     /// Indent only in inner namespaces (nested in other namespaces).
return AccessModifierOffset == R.AccessModifierOffset &&
            ExperimentalAutoDetectBinPacking ==
                R.ExperimentalAutoDetectBinPacking &&
            ForEachMacros == R.ForEachMacros &&
            IndentCaseLabels == R.IndentCaseLabels &&
            IndentWidth == R.IndentWidth && Language == R.Language &&
            IndentWrappedFunctionNames == R.IndentWrappedFunctionNames &&
            KeepEmptyLinesAtTheStartOfBlocks ==
                R.KeepEmptyLinesAtTheStartOfBlocks &&
+           MacroBlockBegin == R.MacroBlockBegin &&
+           MacroBlockEnd == R.MacroBlockEnd &&
            MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep &&
            NamespaceIndentation == R.NamespaceIndentation &&
            ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
            ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
            ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
            PenaltyBreakBeforeFirstCallParameter ==
                R.PenaltyBreakBeforeFirstCallParameter &&
            PenaltyBreakComment == R.PenaltyBreakComment &&

cfe/trunk/lib/Format/Format.cpp

 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
 #include "llvm/Support/YAMLTraits.h"
 #include <queue>
 #include <string>
 
 #define DEBUG_TYPE "format-formatter"
 
 using clang::format::FormatStyle;
 
IO.mapOptional("ExperimentalAutoDetectBinPacking",
                    Style.ExperimentalAutoDetectBinPacking);
     IO.mapOptional("ForEachMacros", Style.ForEachMacros);
     IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
     IO.mapOptional("IndentWidth", Style.IndentWidth);
     IO.mapOptional("IndentWrappedFunctionNames",
                    Style.IndentWrappedFunctionNames);
     IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
                    Style.KeepEmptyLinesAtTheStartOfBlocks);
+    IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
+    IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
     IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
     IO.mapOptional("NamespaceIndentation", Style.NamespaceIndentation);
     IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
     IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
     IO.mapOptional("ObjCSpaceBeforeProtocolList",
                    Style.ObjCSpaceBeforeProtocolList);
     IO.mapOptional("PenaltyBreakBeforeFirstCallParameter",
                    Style.PenaltyBreakBeforeFirstCallParameter);
FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
   } else {
     ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
     ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
     ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
     ChromiumStyle.AllowShortLoopsOnASingleLine = false;
     ChromiumStyle.BinPackParameters = false;
     ChromiumStyle.DerivePointerAlignment = false;
   }
+  ChromiumStyle.MacroBlockBegin = "^IPC_BEGIN_MESSAGE_MAP$";
+  ChromiumStyle.MacroBlockBegin = "^IPC_END_MESSAGE_MAP$";
   return ChromiumStyle;
 }
 
 FormatStyle getMozillaStyle() {
   FormatStyle MozillaStyle = getLLVMStyle();
   MozillaStyle.AllowAllParametersOfDeclarationOnNextLine = false;
   MozillaStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   MozillaStyle.AlwaysBreakAfterDefinitionReturnType =

 class FormatTokenLexer {
 public:
   FormatTokenLexer(SourceManager &SourceMgr, FileID ID, FormatStyle &Style,
                    encoding::Encoding Encoding)
       : FormatTok(nullptr), IsFirstToken(true), GreaterStashed(false),
         LessStashed(false), Column(0), TrailingWhitespace(0),
         SourceMgr(SourceMgr), ID(ID), Style(Style),
         IdentTable(getFormattingLangOpts(Style)), Keywords(IdentTable),
-        Encoding(Encoding), FirstInLineIndex(0), FormattingDisabled(false) {
+        Encoding(Encoding), FirstInLineIndex(0), FormattingDisabled(false),
+        MacroBlockBeginRegex(Style.MacroBlockBegin),
+        MacroBlockEndRegex(Style.MacroBlockEnd) {
     Lex.reset(new Lexer(ID, SourceMgr.getBuffer(ID), SourceMgr,
                         getFormattingLangOpts(Style)));
     Lex->SetKeepWhitespaceMode(true);
 
     for (const std::string &ForEachMacro : Style.ForEachMacros)
       ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
     std::sort(ForEachMacros.begin(), ForEachMacros.end());
   }
if (FirstNewlinePos == StringRef::npos) {
       // The last line of the token always starts in column 0.
       // Thus, the length can be precomputed even in the presence of tabs.
       FormatTok->LastLineColumnWidth = encoding::columnWidthWithTabs(
           Text.substr(Text.find_last_of('\n') + 1), 0, Style.TabWidth,
           Encoding);
       Column = FormatTok->LastLineColumnWidth;
     }
 
+    if (Style.Language == FormatStyle::LK_Cpp) {
       if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
             Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
                 tok::pp_define) &&
           std::find(ForEachMacros.begin(), ForEachMacros.end(),
-                  FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end())
+                    FormatTok->Tok.getIdentifierInfo()) != ForEachMacros.end()) {
         FormatTok->Type = TT_ForEachMacro;
+      } else if (FormatTok->is(tok::identifier)) {
+        if (MacroBlockBeginRegex.match(Text)) {
+          FormatTok->Type = TT_MacroBlockBegin;
+        } else if (MacroBlockEndRegex.match(Text)) {
+          FormatTok->Type = TT_MacroBlockEnd;
+        }
+      }
+    }
 
     return FormatTok;
   }
 
   FormatToken *FormatTok;
   bool IsFirstToken;
   bool GreaterStashed, LessStashed;
   unsigned Column;
   unsigned TrailingWhitespace;
   std::unique_ptr<Lexer> Lex;
   SourceManager &SourceMgr;
   FileID ID;
   FormatStyle &Style;
   IdentifierTable IdentTable;
   AdditionalKeywords Keywords;
   encoding::Encoding Encoding;
   llvm::SpecificBumpPtrAllocator<FormatToken> Allocator;
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector<FormatToken *, 16> Tokens;
   SmallVector<IdentifierInfo *, 8> ForEachMacros;
 
   bool FormattingDisabled;
 
+  llvm::Regex MacroBlockBeginRegex;
+  llvm::Regex MacroBlockEndRegex;
+
   void readRawToken(FormatToken &Tok) {
     Lex->LexFromRawLexer(Tok.Tok);
     Tok.TokenText = StringRef(SourceMgr.getCharacterData(Tok.Tok.getLocation()),
                               Tok.Tok.getLength());
     // For formatting, treat unterminated string literals like normal string
     // literals.
     if (Tok.is(tok::unknown)) {
       if (!Tok.TokenText.empty() && Tok.TokenText[0] == '"') {

cfe/trunk/lib/Format/FormatToken.h

enum TokenType {
   TT_JsComputedPropertyName,
   TT_JsFatArrow,
   TT_JsTypeColon,
   TT_JsTypeOptionalQuestion,
   TT_LambdaArrow,
   TT_LambdaLSquare,
   TT_LeadingJavaAnnotation,
   TT_LineComment,
+  TT_MacroBlockBegin,
+  TT_MacroBlockEnd,
   TT_ObjCBlockLBrace,
   TT_ObjCBlockLParen,
   TT_ObjCDecl,
   TT_ObjCForIn,
   TT_ObjCMethodExpr,
   TT_ObjCMethodSpecifier,
   TT_ObjCProperty,
   TT_ObjCStringLiteral,

cfe/trunk/lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseFile() {
   // Make sure to format the remaining tokens.
   flushComments(true);
   addUnwrappedLine();
 }
 
 void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
   bool SwitchLabelEncountered = false;
   do {
-    switch (FormatTok->Tok.getKind()) {
+    tok::TokenKind kind = FormatTok->Tok.getKind();
+    if (FormatTok->Type == TT_MacroBlockBegin) {
+      kind = tok::l_brace;
+    } else if (FormatTok->Type == TT_MacroBlockEnd) {
+      kind = tok::r_brace;
+    }
+
+    switch (kind) {
     case tok::comment:
       nextToken();
       addUnwrappedLine();
       break;
     case tok::l_brace:
       // FIXME: Add parameter whether this can happen - if this happens, we must
       // be in a non-declaration context.
       parseBlock(/*MustBeDeclaration=*/false);
if (LBraceStack[i]->BlockKind == BK_Unknown)
       LBraceStack[i]->BlockKind = BK_Block;
   }
 
   FormatTok = Tokens->setPosition(StoredPosition);
 }
 
 void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
                                      bool MunchSemi) {
-  assert(FormatTok->Tok.is(tok::l_brace) && "'{' expected");
+  assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
+         "'{' or macro block token expected");
+  const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
+
   unsigned InitialLevel = Line->Level;
   nextToken();
 
+  if (MacroBlock && FormatTok->is(tok::l_paren))
+    parseParens();
+
   addUnwrappedLine();
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                           MustBeDeclaration);
   if (AddLevel)
     ++Line->Level;
   parseLevel(/*HasOpeningBrace=*/true);
 
-  if (!FormatTok->Tok.is(tok::r_brace)) {
+  if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
+                 : !FormatTok->is(tok::r_brace)) {
     Line->Level = InitialLevel;
     return;
   }
 
   nextToken(); // Munch the closing brace.
+
+  if (MacroBlock && FormatTok->is(tok::l_paren))
+    parseParens();
+
   if (MunchSemi && FormatTok->Tok.is(tok::semi))
     nextToken();
   Line->Level = InitialLevel;
 }
 
 static bool isGoogScope(const UnwrappedLine &Line) {
   // FIXME: Closure-library specific stuff should not be hard-coded but be
   // configurable.
if (Style.Language == FormatStyle::LK_JavaScript) {
       return;
     }
     break;
   case tok::identifier:
     if (FormatTok->is(TT_ForEachMacro)) {
       parseForOrWhileLoop();
       return;
     }
+    if (FormatTok->is(TT_MacroBlockBegin)) {
+      parseBlock(/*MustBeDeclaration=*/false, /*AddLevel=*/true,
+                 /*MunchSemi=*/false);
+      return;
+    }
     if (Style.Language == FormatStyle::LK_JavaScript &&
         FormatTok->is(Keywords.kw_import)) {
       parseJavaScriptEs6ImportExport();
       return;
     }
     if (FormatTok->is(Keywords.kw_signals)) {
       nextToken();
       if (FormatTok->is(tok::colon)) {
case tok::l_brace:
       // Otherwise this was a braced init list, and the structural
       // element continues.
       break;
     case tok::kw_try:
       // We arrive here when parsing function-try blocks.
       parseTryCatch();
       return;
     case tok::identifier: {
+      if (FormatTok->is(TT_MacroBlockEnd)) {
+        addUnwrappedLine();
+        return;
+      }
+
       // Parse function literal unless 'function' is the first token in a line
       // in which case this should be treated as a free-standing function.
       if (Style.Language == FormatStyle::LK_JavaScript &&
           FormatTok->is(Keywords.kw_function) && Line->Tokens.size() > 0) {
         tryToParseJSFunction();
         break;
       }
       if ((Style.Language == FormatStyle::LK_JavaScript ||

cfe/trunk/unittests/Format/FormatTest.cpp

 }
 
 TEST_F(FormatTest, PutEmptyBlocksIntoOneLine) {
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
 }
 
+TEST_F(FormatTest, FormatBeginBlockEndMacros) {
+  FormatStyle Style = getLLVMStyle();
+  Style.MacroBlockBegin = "^[A-Z_]+_BEGIN$";
+  Style.MacroBlockEnd = "^[A-Z_]+_END$";
+  verifyFormat("FOO_BEGIN\n"
+               "  FOO_ENTRY\n"
+               "FOO_END", Style);
+  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);
+}
+
 //===----------------------------------------------------------------------===//
 // Line break tests.
 //===----------------------------------------------------------------------===//
 
 TEST_F(FormatTest, PreventConfusingIndents) {
   verifyFormat(
       "void f() {\n"
       "  SomeLongMethodName(SomeReallyLongMethod(CallOtherReallyLongMethod(\n"