[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf21c704553a8: clang-format: Add 
ControlStatementsExceptForEachMacros option to… (authored by DaanDeMeyer).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2900,6 +2900,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -329,6 +329,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1983,6 +1983,17 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+/// Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+/// ForEach macros. This is useful in projects where ForEach macros are 
+/// treated as function calls instead of control statements. 
+/// \code
+///void f() {
+///  Q_FOREACH(...) {
+///f();
+///  }
+///}
+/// \endcode
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2336,6 +2336,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros. This is useful in projects where ForEach macros are
+treated as function calls instead of control statements.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before opening parentheses only if the parentheses are not
 empty i.e. '()'


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer added a comment.

I pushed the commit to github. Thanks for the help!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D78869#2022850 , @mitchell-stellar 
wrote:

> I often:


I develop and run on windows (using a cygwin shell), but with visual studio 
compiler (configured with CMake for ninja)

  0.5  after doing git add of the files, I do `git clang-format` just to be 
sure  (adding any files that are modified)

> 1. Commit the change to my local repo, following the style of previous commit 
> messages for clang-format.

So I have a script which gets the commit message

  get_commit_message.sh
  
  echo '{ "revision_id": '${1:1}' }' | arc call-conduit --conduit-uri 
https://reviews.llvm.org/ --conduit-token  
differential.getcommitmessage | /usr/bin/jq -r ".response"
  ---

This will fetch the commit message from Phabricator that I should use. (you 
need to get a phabricator-token from your profile->settings)

  git fetch  && git pull --rebase --autostash

> 2. Build and run the clang-format tests and make sure they pass.
> 3. Pull, rebase, and push. (Linear history is enforced.)
> 4. Build and run the clang-format tests again just in case. (Optional)



5. I try to run the lit tests too. (in clang/tests/Format)
6. I check the documentation if any rst files are changed  ( 
/usr/bin/sphinx-build -n ./docs ./html)

  // assuming no new changes.. git push


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I often:

1. Commit the change to my local repo, following the style of previous commit 
messages for clang-format.
2. Build and run the clang-format tests and make sure they pass.
3. Pull, rebase, and push. (Linear history is enforced.)
4. Build and run the clang-format tests again just in case. (Optional)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer added a comment.

I should have commit access now. Is there any documentation on how exactly to 
do a merge? Do I just add the commit to master and push? I apologize if this 
seems obvious, I just want to make sure I'm not inadvertently breaking 
something.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D78869#2019933 , @DaanDeMeyer wrote:

> (I don't have commit access so it'd be great if someone could merge this for 
> me, at least that's how my other contributions have been handled)


You really should think about getting commit access especially as this isn't 
your first patch, I found it the best way to gain experience and its nice to 
have new people on board with us.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer added a comment.

(I don't have commit access so it'd be great if someone could merge this for 
me, at least that's how my other contributions have been handled)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

If there is no additional comments from others then I'm fine giving this a 
LGTM, if it means we can encourage other big projects to go full clang-format


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:982
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",

UNKNOWN_FOREACH?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I've added a couple of others reviewers to comment, I don't personally mind as 
I don't use the ForEach Macros.  It feels kind of funny to me to differentiate 
just the for macros from for,if,while,for..  (I guess I'm struggling to 
understand why you wouldn't just add the space other than the churn you'll get)

But I won't block it if others are ok with it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer marked 3 inline comments as done.
DaanDeMeyer added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2311
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)

MyDeveloperDay wrote:
> Normally this would be generated from the Format.h after running the dump 
> style script in tools, did you?
I didn't think the docs were automatically generated. I added a comment to 
Format.h and re-generated the docs using the dump_format_style script.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer updated this revision to Diff 260161.
DaanDeMeyer added a comment.

- Moved docs to Format.h and re-generated rst file using dump_format_style.py
- Expanded comment with an explanation for why you might want to not have a 
space before the parens of a ForEach macro

This isn't explicitly mentioned in systemd's style guide. I simply did a regex 
search over systemd's codebase. Searching (using regex) for ForEach macro 
usages with a space before the parens returns 7 matches over the entire 
codebase. Searching for ForEach macro usages without a space before the parens 
returns 1753 matches. If needed, I can make a systemd PR that proposes 
explicitly adding this to the style guide.

I only added the relevant changes produced by dump_format_style to this 
revision but there were some other changes produced as well (just to let you 
know).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -327,6 +327,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1954,6 +1954,17 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+/// Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+/// ForEach macros. This is useful in projects where ForEach macros are 
+/// treated as function calls instead of control statements. 
+/// \code
+///void f() {
+///  Q_FOREACH(...) {
+///f();
+///  }
+///}
+/// \endcode
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2308,6 +2308,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros. This is useful in projects where ForEach macros are
+treated as function calls instead of control statements.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before 

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Could you add to the review description a link to the systemd style guide that 
says it should be like this?




Comment at: clang/docs/ClangFormatStyleOptions.rst:2311
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)

Normally this would be generated from the Format.h after running the dump style 
script in tools, did you?



Comment at: clang/include/clang/Format/Format.h:1958
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'

can you move this down below this comment which applies to 
`SBPO_ControlStatements` and add your own comment explaining why you might NOT 
want them in front of the ForeachEach Macros which I have to say I'm confused 
why you wouldn't but then its not for me to understand you style requirements.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



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

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer updated this revision to Diff 260157.
DaanDeMeyer added a comment.

Fixed formatting


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -327,6 +327,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1954,6 +1954,7 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2308,6 +2308,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before opening parentheses only if the parentheses are not
 empty i.e. '()'


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,17 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Daan De Meyer via Phabricator via cfe-commits]

DaanDeMeyer created this revision.
DaanDeMeyer added a reviewer: clang-format.
DaanDeMeyer added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

systemd  recently added a clang-format 
file. One issue I encountered in using clang-format on systemd is that systemd 
does not add a space before the parens of their foreach macros but clang-format 
always adds a space. This does not seem to be configurable in clang-format. 
This revision adds the ControlStatementsExceptForEachMacros option to 
SpaceBeforeParens which puts a space before all control statement parens except 
ForEach macros. This drastically reduces the amount of changes when running 
clang-format on systemd's source code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78869

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,16 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "  BOOST_FOREACH(Item *item, itemlist) {}\n"
+   "  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
+   "}", Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2889,6 +2889,10 @@
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
+if (Style.SpaceBeforeParens ==
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros &&
+Left.is(TT_ForEachMacro))
+  return false;
 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) ||
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -327,6 +327,8 @@
 IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
 IO.enumCase(Value, "ControlStatements",
 FormatStyle::SBPO_ControlStatements);
+IO.enumCase(Value, "ControlStatementsExceptForEachMacros",
+FormatStyle::SBPO_ControlStatementsExceptForEachMacros);
 IO.enumCase(Value, "NonEmptyParentheses",
 FormatStyle::SBPO_NonEmptyParentheses);
 IO.enumCase(Value, "Always", FormatStyle::SBPO_Always);
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1954,6 +1954,7 @@
 ///}
 /// \endcode
 SBPO_ControlStatements,
+SBPO_ControlStatementsExceptForEachMacros,
 /// Put a space before opening parentheses only if the parentheses are not
 /// empty i.e. '()'
 /// \code
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2308,6 +2308,19 @@
  }
}
 
+  * ``SBPO_ControlStatementsExceptForEachMacros`` (in configuration: 
+``ControlStatementsExceptForEachMacros``)
+Same as ``SBPO_ControlStatements`` except this option doesn't apply to
+ForEach macros.
+
+.. code-block:: c++
+
+   void f() {
+ Q_FOREACH(...) {
+   f();
+ }
+   }
+
   * ``SBPO_NonEmptyParentheses`` (in configuration: ``NonEmptyParentheses``)
 Put a space before opening parentheses only if the parentheses are not
 empty i.e. '()'


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -972,6 +972,16 @@
"  UNKNOWN_FORACH(Item * item, itemlist) {}\n"
"}");
 
+  FormatStyle Style = getLLVMStyle();
+  Style.SpaceBeforeParens =
+  FormatStyle::SBPO_ControlStatementsExceptForEachMacros;
+  verifyFormat("void f() {\n"
+   "  foreach(Item *item, itemlist) {}\n"
+