krasimir added a comment.
Patch looks good, but I also would like to see it splited. I would suggest to
first get the statement macro part in, which requires less code. Then we can
put the namespace macros on top of that. I really like the generality of this
approach and would want to also add support for `class` macros eventually.
================
Comment at: lib/Format/FormatTokenLexer.cpp:641
+ auto it = std::find(Macros.begin(), Macros.end(),
+ FormatTok->Tok.getIdentifierInfo());
if (!(Tokens.size() > 0 && Tokens.back()->Tok.getIdentifierInfo() &&
----------------
Please move this inside the following `if` statement, so that we only perform
the search when we see a `tok::pp_define`.
================
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:56
+ assert(Tok && Tok->is(tok::l_paren) && "expected an opening parenthesis");
+ Tok = Tok ? Tok->getNextNonComment() : nullptr;
+ while (Tok && !Tok->isOneOf(tok::r_paren, tok::comma)) {
----------------
I don't understand why you have a `Tok ? ...` after you `assert(Tok && ...)`?
================
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:82
+ text += ")";
+ // close brace
if (AddNewline)
----------------
What does this comment refer to? If it's about the line above, consider moving
it up.
================
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:100
+ StringRef NamespaceTokenText = Groups.size() > 4 ? Groups[4] : "";
+ // The name of the macro must be used
+ if (NamespaceTokenText != NamespaceTok->TokenText)
----------------
nit: end comment with `.`
================
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:105
+ !kNamespaceCommentPattern.match(Comment->TokenText, &Groups)) {
+ // Comment does not match regex
+ return false;
----------------
nit: end comment with `.`
================
Comment at: lib/Format/NamespaceEndCommentsFixer.cpp:155
const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First;
- // Detect "(inline)? namespace" in the beginning of a line.
- if (NamespaceTok->is(tok::kw_inline))
- NamespaceTok = NamespaceTok->getNextNonComment();
- if (!NamespaceTok || NamespaceTok->isNot(tok::kw_namespace))
- return nullptr;
- return NamespaceTok;
+ return NamespaceTok->getNamespaceToken();
+}
----------------
What happened to the old `// Detect "(inline)? namespace" in the beginning of a
line.`
================
Comment at: unittests/Format/FormatTest.cpp:1588
+ Style.NamespaceIndentation = FormatStyle::NI_All;
+ verifyFormat("TESTSUITE(A) {\n"
+ " int foo();\n"
----------------
Hm, what would happen if you have a namespace macro with two or more parameters?
https://reviews.llvm.org/D33440
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
