[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-13 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz updated this revision to Diff 115057.
Typz added a comment.

Split diff: handle only statements in here, namespace macros will be moved to 
another one.


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -2385,6 +2385,26 @@
getLLVMStyleWithColumns(40)));
 
   verifyFormat("MACRO(>)");
+
+  // Some macros contain an implicit semicolon
+  FormatStyle Style = getLLVMStyle();
+  Style.StatementMacros.push_back("FOO");
+  verifyFormat("FOO(a) int b = 0;");
+  verifyFormat("FOO(a)\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(a);\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO(argc, argv, \"4.0.2\")\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO()\n"
+   "int b = 0;",
+   Style);
+  verifyFormat("FOO\n"
+   "int b = 0;",
+   Style);
 }
 
 TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) {
@@ -10185,6 +10205,12 @@
   CHECK_PARSE("ForEachMacros: [BOOST_FOREACH, Q_FOREACH]", ForEachMacros,
   BoostAndQForeach);
 
+  Style.StatementMacros.clear();
+  CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros,
+  std::vector{"QUNUSED"});
+  CHECK_PARSE("StatementMacros: [QUNUSED, QT_REQUIRE_VERSION]", StatementMacros,
+  std::vector({"QUNUSED", "QT_REQUIRE_VERSION"}));
+
   Style.IncludeCategories.clear();
   std::vector ExpectedCategories = {{"abc/.*", 2},
   {".*", 1}};
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1089,6 +1089,15 @@
 return;
   }
 }
+if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+  nextToken();
+  if (FormatTok->is(tok::l_paren))
+parseParens();
+  if (FormatTok->is(tok::semi))
+nextToken();
+  addUnwrappedLine();
+  return;
+}
 // In all other cases, parse the declaration.
 break;
   default:
Index: lib/Format/FormatTokenLexer.h
===
--- lib/Format/FormatTokenLexer.h
+++ lib/Format/FormatTokenLexer.h
@@ -97,7 +97,29 @@
   // Index (in 'Tokens') of the last token that starts a new line.
   unsigned FirstInLineIndex;
   SmallVector Tokens;
-  SmallVector ForEachMacros;
+
+  struct MacroInfo {
+MacroInfo() : Identifier(NULL), TokType(TT_Unknown) {}
+MacroInfo(IdentifierInfo *Identifier, TokenType TokType)
+: Identifier(Identifier), TokType(TokType) {}
+
+IdentifierInfo *Identifier;
+TokenType TokType;
+
+bool operator==(const MacroInfo &Other) const {
+  return Identifier == Other.Identifier;
+}
+bool operator==(const IdentifierInfo *Other) const {
+  return Identifier == Other;
+}
+bool operator<(const MacroInfo &Other) const {
+  return Identifier < Other.Identifier;
+}
+bool operator<(const IdentifierInfo *Other) const {
+  return Identifier < Other;
+}
+  };
+  SmallVector Macros;
 
   bool FormattingDisabled;
 
Index: lib/Format/FormatTokenLexer.cpp
===
--- lib/Format/FormatTokenLexer.cpp
+++ lib/Format/FormatTokenLexer.cpp
@@ -37,8 +37,10 @@
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string &ForEachMacro : Style.ForEachMacros)
-ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
-  std::sort(ForEachMacros.begin(), ForEachMacros.end());
+Macros.emplace_back(&IdentTable.get(ForEachMacro), TT_ForEachMacro);
+  for (const std::string &StatementMacro : Style.StatementMacros)
+Macros.emplace_back(&IdentTable.get(StatementMacro), TT_StatementMacro);
+  std::sort(Macros.begin(), Macros.end());
 }
 
 ArrayRef FormatTokenLexer::lex() {
@@ -633,12 +635,13 @@
   }
 
   if (Style.isCpp()) {
+decltype(Macros)::iterator it;
 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->Type = TT_ForEachMacro;
+(it = std::find(Macros.begin(), Macros.end(),
+FormatTok->Tok.getIdentifierInfo())) != Macros.end()) {
+  FormatTok->Type = it->TokType;
 } else if (FormatTok->is(tok::identif

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-13 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz updated this revision to Diff 115054.
Typz marked 2 inline comments as done.
Typz added a comment.

Fix review comments, before splitting the commit.


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-13 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz marked 8 inline comments as done.
Typz added inline comments.



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();
+}

krasimir wrote:
> What happened to the old `// Detect "(inline)? namespace" in the beginning of 
> a line.`
Moved to `FormatToken::getNamespaceToken()`



Comment at: unittests/Format/FormatTest.cpp:1588
+  Style.NamespaceIndentation = FormatStyle::NI_All;
+  verifyFormat("TESTSUITE(A) {\n"
+   "  int foo();\n"

krasimir wrote:
> Hm, what would happen if you have a namespace macro with two or more 
> parameters?
only the first argument is used, as seen in previous test case.


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-13 Thread Krasimir Georgiev via Phabricator via cfe-commits 
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

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-12 Thread Daniel Jasper via Phabricator via cfe-commits 
djasper added a comment.

I'd still prefer individual patches for each of these changes. If the code 
review system or VCS make it hard for you to deal with two adjacent changes 
this way, do them in sequence.

Adding Manuel as a reviewer who has a longer term idea on how to handle macros.


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-12 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz updated this revision to Diff 114783.
Typz added a comment.

Rebase to master to fix merge issue


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-09-11 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-07-18 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz added a comment.

t>>! In https://reviews.llvm.org/D33440#812645, @djasper wrote:

> So, there are two things in this patch: Statement macros and namespace 
> macros. Lets break this out and handle them individually. They really aren't 
> related that much.

Indeed, the only "relation" is the implementation, which now uses the exact 
same list (internally) to match all macros. Phabricator makes it very difficult 
to work with related changes pushed as multiple reviews, so I ended up merging 
the two features.

> Statement macros:
>  I think clang-format's handling here is good enough. clang-format does not 
> insert the line break, but it also doesn't remove it. I am not 100% sure 
> here, so I an be convinced. But I want to understand the use cases better. Do 
> you expect people to run into this frequently? I am essentially trying to 
> understand whether the cost of an extra option is worth the benefit it is 
> giving.

This happens relatively often, for example when fixing "unused parameter 
warning" on an inlined function: ``int f(int a) { return 0; }`` often gets 
fixed to ``int f(int a) { Q_UNUSED(a) return 0; }`` and clang-format does not 
fix the formatting...

> Namespace macros:
>  How important are the automatic closing comments to you? I'd say that we 
> should punt on that and leave it to the user to fix comments of these. And 
> then, we could try to make the things we already have in MacroBlockBegin 
> detect whether it ends with an opening brace and not need an extra list here. 
> What do you think?

This is not just about automatic closing comments, there are may differences: 
indentation of namespaces, 'compacting' of namespaces when `CompactNamespaces` 
is used, detection of inline functions (for putting on a single line with 
SFS_Inline), handling of empty blocks (i.e. use 
`BraceWrappingFlags.SplitEmptyNamespace`)...


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-07-18 Thread Daniel Jasper via Phabricator via cfe-commits 
djasper added a comment.

So, there are two things in this patch: Statement macros and namespace macros. 
Lets break this out and handle them individually. They really aren't related 
that much.

Statement macros:
I think clang-format's handling here is good enough. clang-format does not 
insert the line break, but it also doesn't remove it. I am not 100% sure here, 
so I an be convinced. But I want to understand the use cases better. Do you 
expect people to run into this frequently? I am essentially trying to 
understand whether the cost of an extra option is worth the benefit it is 
giving.

Namespace macros:
How important are the automatic closing comments to you? I'd say that we should 
punt on that and leave it to the user to fix comments of these. And then, we 
could try to make the things we already have in MacroBlockBegin detect whether 
it ends with an opening brace and not need an extra list here. What do you 
think?




Comment at: unittests/Format/FormatTest.cpp:1483
 
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",

What's the difference here?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-07-13 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-07-05 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-06-28 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz added a comment.

ping?


https://reviews.llvm.org/D33440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33440: clang-format: better handle statement and namespace macros

2017-06-26 Thread Francois Ferrand via Phabricator via cfe-commits 
Typz updated this revision to Diff 103937.
Typz added a comment.

Fix typo


https://reviews.llvm.org/D33440

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/FormatToken.h
  lib/Format/FormatTokenLexer.cpp
  lib/Format/FormatTokenLexer.h
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -53,6 +53,7 @@
 "  int i;\n"
 "  int j;\n"
 "}"));
+
   EXPECT_EQ("namespace {\n"
 "  int i;\n"
 "  int j;\n"
@@ -249,6 +250,85 @@
 "// unrelated"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, AddsMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("inline TESTSUITE(A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A)",
+fixNamespaceEndComments("TESTSUITE(::A) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(::A::B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(::A::B)",
+fixNamespaceEndComments("TESTSUITE(/**/::/**/A/**/::/**/B/**/) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(A)",
+fixNamespaceEndComments("TESTSUITE(A, B) {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+  EXPECT_EQ("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}// TESTSUITE(\"Test1\")",
+fixNamespaceEndComments("TESTSUITE(\"Test1\") {\n"
+"  int i;\n"
+"  int j;\n"
+"}",
+Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
 "  int i;\n"
@@ -381,6 +461,54 @@
 "}; /* unnamed namespace */"));
 }
 
+TEST_F(NamespaceEndCommentsFixerTest, KeepsValidMacroEndComment) {
+  FormatStyle Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("TESTSUITE");
+
+  EXPECT_EQ("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+fixNamespaceEndComments("TESTSUITE() {\n"
+"  int i;\n"
+"} // end anonymous TESTSUITE()",
+Style));
+  EXPECT_EQ("TESTSUITE(A) {\n"
+"  int i;\n"
+