================
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/
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
