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

[Daniel Jasper]

================
Comment at: include/clang/Format/Format.h:159
@@ -158,1 +158,3 @@
 
+  /// \brief A regular expression matching macros that start a block.
+  std::string BlockBeginMacros;
----------------
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?

================
Comment at: lib/Format/Format.cpp:460
@@ -456,3 +459,3 @@
 
 FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
   FormatStyle ChromiumStyle = getGoogleStyle(Language);
----------------
Can you set values for the new flags here, so it properly handles IPC_BEGIN_MESSAGE_MAP and IPC_END_MESSAGE_MAP?

================
Comment at: lib/Format/Format.cpp:477
@@ -473,3 +476,3 @@
 
 FormatStyle getMozillaStyle() {
   FormatStyle MozillaStyle = getLLVMStyle();
----------------
Do you have good defaults here?

================
Comment at: lib/Format/Format.cpp:1168-1180
@@ -1162,8 +1167,15 @@
 
     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 (BlockBeginMacrosRegex.match(Text)) {
+        FormatTok->Type = TT_MacroBlockBegin;
+      } else if (BlockEndMacrosRegex.match(Text)) {
+        FormatTok->Type = TT_MacroBlockEnd;
+      }
+    }
 
----------------
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.

================
Comment at: lib/Format/UnwrappedLineParser.cpp:404
@@ +403,3 @@
+  const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
+  assert((FormatTok->is(tok::l_brace) || MacroBlock) &&
+         "'{' or macro block token expected");
----------------
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);

================
Comment at: lib/Format/UnwrappedLineParser.cpp:421-422
@@ -407,3 +420,4 @@
 
-  if (!FormatTok->Tok.is(tok::r_brace)) {
+  if (!(MacroBlock ? FormatTok->is(TT_MacroBlockEnd)
+                   : FormatTok->is(tok::r_brace))) {
     Line->Level = InitialLevel;
----------------
Can you pull the "!" into the ternary expression? Otherwise this is just hard to read for me (can't really explain why).

http://reviews.llvm.org/D10840

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/