[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud updated this revision to Diff 384679.
crayroud added a comment.

Fix ordering of SpaceBeforeParensCustom options


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

https://reviews.llvm.org/D110833

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.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
@@ -14133,6 +14133,173 @@
   verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
+
+  FormatStyle SpaceControlStatements = getLLVMStyle();
+  SpaceControlStatements.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceControlStatements.SpaceBeforeParensOptions.AfterControlStatements = true;
+
+  verifyFormat("while (true)\n"
+   "  continue;",
+   SpaceControlStatements);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SpaceControlStatements);
+  verifyFormat("for (;;) {\n"
+   "  do_something();\n"
+   "}",
+   SpaceControlStatements);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SpaceControlStatements);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SpaceControlStatements);
+
+  FormatStyle SpaceFuncDecl = getLLVMStyle();
+  SpaceFuncDecl.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDecl.SpaceBeforeParensOptions.AfterFunctionDeclarationName = true;
+
+  verifyFormat("int f ();", SpaceFuncDecl);
+  verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
+  verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("#define A(x) x", SpaceFuncDecl);
+  verifyFormat("#define A (x) x", SpaceFuncDecl);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDecl);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDecl);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> decltype(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> typeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> _Atomic(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> __underlying_type(x);", SpaceFuncDecl);
+  verifyFormat("int f (T x) noexcept(x.create());", SpaceFuncDecl);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDecl);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDecl);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDecl);
+  verifyFormat("int f () throw(Deprecated);", SpaceFuncDecl);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDecl);
+  verifyFormat("T A::operator() ();", SpaceFuncDecl);
+  verifyFormat("X A::operator++ (T);", SpaceFuncDecl);
+  verifyFormat("T A::operator()() {}", SpaceFuncDecl);
+  verifyFormat("auto lambda = []() { return 0; };", SpaceFuncDecl);
+  verifyFormat("int x = int(y);", SpaceFuncDecl);
+  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SpaceFuncDecl);
+
+  FormatStyle SpaceFuncDef = getLLVMStyle();
+  SpaceFuncDef.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDef.SpaceBeforeParensOptions.AfterFunctionDefinitionName = true;
+
+  verifyFormat("int f();", SpaceFuncDef);
+  verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("#define A(x) x", SpaceFuncDef);
+  verifyFormat("#define A (x) x", SpaceFuncDef);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDef);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDef);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> decltype(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> typeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> _Atomic(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", SpaceFuncDef);
+  verifyFormat("int f(T x) noexcept(x.create());", SpaceFuncDef);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDef);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDef);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDef);
+  verifyFormat("int f() throw(Deprecated);", SpaceFuncDef);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
+  verifyFormat("T A::operator()();", SpaceFuncDef);
+  

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud marked an inline comment as done.
crayroud added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:14275
+  verifyFormat("A::A() : a (1) {}", SomeSpace2);
+  verifyFormat("void f() __attribute__ ((asdf));", SomeSpace2);
+  verifyFormat("*( + 1);\n"

HazardyKnusperkeks wrote:
> Is this really desired?
It is allowed to have a space after `__attribute__` and the behaviour is the 
same with `SpaceBeforeParens: NonEmptyParentheses`.


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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud updated this revision to Diff 384352.
crayroud marked an inline comment as done.
crayroud added a comment.

Reorder the options


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

https://reviews.llvm.org/D110833

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.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
@@ -14133,6 +14133,173 @@
   verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
+
+  FormatStyle SpaceControlStatements = getLLVMStyle();
+  SpaceControlStatements.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceControlStatements.SpaceBeforeParensOptions.AfterControlStatements = true;
+
+  verifyFormat("while (true)\n"
+   "  continue;",
+   SpaceControlStatements);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SpaceControlStatements);
+  verifyFormat("for (;;) {\n"
+   "  do_something();\n"
+   "}",
+   SpaceControlStatements);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SpaceControlStatements);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SpaceControlStatements);
+
+  FormatStyle SpaceFuncDecl = getLLVMStyle();
+  SpaceFuncDecl.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDecl.SpaceBeforeParensOptions.AfterFunctionDeclarationName = true;
+
+  verifyFormat("int f ();", SpaceFuncDecl);
+  verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
+  verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("#define A(x) x", SpaceFuncDecl);
+  verifyFormat("#define A (x) x", SpaceFuncDecl);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDecl);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDecl);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> decltype(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> typeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> _Atomic(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> __underlying_type(x);", SpaceFuncDecl);
+  verifyFormat("int f (T x) noexcept(x.create());", SpaceFuncDecl);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDecl);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDecl);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDecl);
+  verifyFormat("int f () throw(Deprecated);", SpaceFuncDecl);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDecl);
+  verifyFormat("T A::operator() ();", SpaceFuncDecl);
+  verifyFormat("X A::operator++ (T);", SpaceFuncDecl);
+  verifyFormat("T A::operator()() {}", SpaceFuncDecl);
+  verifyFormat("auto lambda = []() { return 0; };", SpaceFuncDecl);
+  verifyFormat("int x = int(y);", SpaceFuncDecl);
+  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SpaceFuncDecl);
+
+  FormatStyle SpaceFuncDef = getLLVMStyle();
+  SpaceFuncDef.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDef.SpaceBeforeParensOptions.AfterFunctionDefinitionName = true;
+
+  verifyFormat("int f();", SpaceFuncDef);
+  verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("#define A(x) x", SpaceFuncDef);
+  verifyFormat("#define A (x) x", SpaceFuncDef);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDef);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDef);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> decltype(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> typeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> _Atomic(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", SpaceFuncDef);
+  verifyFormat("int f(T x) noexcept(x.create());", SpaceFuncDef);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDef);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDef);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDef);
+  verifyFormat("int f() throw(Deprecated);", SpaceFuncDef);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
+  verifyFormat("T A::operator()();", 

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud updated this revision to Diff 383995.
crayroud added a comment.

Add a release note into clang/docs/ReleaseNotes.rst


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

https://reviews.llvm.org/D110833

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.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
@@ -14133,6 +14133,173 @@
   verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
+
+  FormatStyle SpaceControlStatements = getLLVMStyle();
+  SpaceControlStatements.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceControlStatements.SpaceBeforeParensOptions.AfterControlStatements = true;
+
+  verifyFormat("while (true)\n"
+   "  continue;",
+   SpaceControlStatements);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SpaceControlStatements);
+  verifyFormat("for (;;) {\n"
+   "  do_something();\n"
+   "}",
+   SpaceControlStatements);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SpaceControlStatements);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SpaceControlStatements);
+
+  FormatStyle SpaceFuncDecl = getLLVMStyle();
+  SpaceFuncDecl.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDecl.SpaceBeforeParensOptions.AfterFunctionDeclarationName = true;
+
+  verifyFormat("int f ();", SpaceFuncDecl);
+  verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
+  verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("#define A(x) x", SpaceFuncDecl);
+  verifyFormat("#define A (x) x", SpaceFuncDecl);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDecl);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDecl);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> decltype(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> typeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> _Atomic(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> __underlying_type(x);", SpaceFuncDecl);
+  verifyFormat("int f (T x) noexcept(x.create());", SpaceFuncDecl);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDecl);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDecl);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDecl);
+  verifyFormat("int f () throw(Deprecated);", SpaceFuncDecl);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDecl);
+  verifyFormat("T A::operator() ();", SpaceFuncDecl);
+  verifyFormat("X A::operator++ (T);", SpaceFuncDecl);
+  verifyFormat("T A::operator()() {}", SpaceFuncDecl);
+  verifyFormat("auto lambda = []() { return 0; };", SpaceFuncDecl);
+  verifyFormat("int x = int(y);", SpaceFuncDecl);
+  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SpaceFuncDecl);
+
+  FormatStyle SpaceFuncDef = getLLVMStyle();
+  SpaceFuncDef.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDef.SpaceBeforeParensOptions.AfterFunctionDefinitionName = true;
+
+  verifyFormat("int f();", SpaceFuncDef);
+  verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("#define A(x) x", SpaceFuncDef);
+  verifyFormat("#define A (x) x", SpaceFuncDef);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDef);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDef);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> decltype(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> typeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> _Atomic(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", SpaceFuncDef);
+  verifyFormat("int f(T x) noexcept(x.create());", SpaceFuncDef);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDef);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDef);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDef);
+  verifyFormat("int f() throw(Deprecated);", SpaceFuncDef);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDef);
+  verifyFormat("T A::operator()();", 

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3469
 return true;
-  if (Right.is(TT_OverloadedOperatorLParen))
-return spaceRequiredBeforeParens(Right);
-  if (Left.is(tok::comma))
+  if (Left.is(tok::comma) && !Right.is(TT_OverloadedOperatorLParen))
 return true;

MyDeveloperDay wrote:
> I'm ever so slightly struggling to see where this case is covered. Could you 
> give me the line number?
Left.is(tok::comma) is used to always add a space after a coma, but we want to 
be able to configure the space after the coma in the following example:

```
bool operator,();
```

Verified by:

```
25201:   verifyFormat("bool operator,();");
```


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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud marked 10 inline comments as done.
crayroud added a comment.

I used the following command to verify my changes on multiple files. Thank you 
for the tip.

  clang-format -verbose -n -files clang/docs/tools/clang-formatted-files.txt


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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud marked 6 inline comments as done.
crayroud added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3708
+SpaceBeforeParens: Custom
+SpaceBeforeParensFlags:
+  AfterFunctionDefinitionName: true

MyDeveloperDay wrote:
> I'm not a massive fan of the use of 'Flags' in the config (I know we use it 
> as the typename), naming things is hard!
SpaceBeforeParensFlags has been renamed to SpaceBeforeParensOptions.



Comment at: clang/include/clang/Format/Format.h:3416
+  /// \version 14
+  SpaceBeforeParensCustom SpaceBeforeParensFlags;
+

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > I'm not a massive fan of the word `Flags` here and thoughts?
> > Yeah, neither, but Options is taken.
> > 
> > But Flags indicate that these will always be booleans, and we know that may 
> > change in the future.
> > 
> > Is it possible the reuse SpaceBeforeParensOptions as struct and still parse 
> > the old enum? (Yes of course, but is it feasible in terms of needed work?)
> but "Options" as in SpaceBeforeParensOptions  is the type not the name so we 
> could use SpaceBeforeParensOptions 
> 
> personally I would change the typename of SpaceBeforeParensOptions  to avoid 
> confusion but its not essential as it would be 
> 
> `SpaceBeforeParensCustom  SpaceBeforeParensOptions;`
> 
> for me I sometimes like using Struct anyway
> 
> `SpaceBeforeParensStruct SpaceBeforeParensOptions;`
SpaceBeforeParensFlags has been renamed to SpaceBeforeParensOptions. And the 
old SpaceBeforeParensOptions enum has been renamed to SpaceBeforeParensStyle.



Comment at: clang/unittests/Format/FormatTest.cpp:14193
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SomeSpace2);
 }

MyDeveloperDay wrote:
> IMHO I think we should see tests for the other combinations of custom (I know 
> it might be repeated)
More tests has been added


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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add options

[Christian Rayroud via Phabricator via cfe-commits]

crayroud updated this revision to Diff 383988.
crayroud retitled this revision from "[clang-format] Refactor SpaceBeforeParens 
to add flags" to "[clang-format] Refactor SpaceBeforeParens to add options".
crayroud edited the summary of this revision.
crayroud added a comment.

- SpaceBeforeParensFlags has been renamed to SpaceBeforeParensOptions
- All common space before parentheses are now grouped
- More tests has been added


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

https://reviews.llvm.org/D110833

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
@@ -14133,6 +14133,173 @@
   verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
+
+  FormatStyle SpaceControlStatements = getLLVMStyle();
+  SpaceControlStatements.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceControlStatements.SpaceBeforeParensOptions.AfterControlStatements = true;
+
+  verifyFormat("while (true)\n"
+   "  continue;",
+   SpaceControlStatements);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SpaceControlStatements);
+  verifyFormat("for (;;) {\n"
+   "  do_something();\n"
+   "}",
+   SpaceControlStatements);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SpaceControlStatements);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SpaceControlStatements);
+
+  FormatStyle SpaceFuncDecl = getLLVMStyle();
+  SpaceFuncDecl.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDecl.SpaceBeforeParensOptions.AfterFunctionDeclarationName = true;
+
+  verifyFormat("int f ();", SpaceFuncDecl);
+  verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
+  verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("#define A(x) x", SpaceFuncDecl);
+  verifyFormat("#define A (x) x", SpaceFuncDecl);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDecl);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDecl);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> decltype(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> typeof(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> _Atomic(x);", SpaceFuncDecl);
+  verifyFormat("auto f (int x) -> __underlying_type(x);", SpaceFuncDecl);
+  verifyFormat("int f (T x) noexcept(x.create());", SpaceFuncDecl);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDecl);
+  verifyFormat("size_t x = alignof(MyType);", SpaceFuncDecl);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SpaceFuncDecl);
+  verifyFormat("int f () throw(Deprecated);", SpaceFuncDecl);
+  verifyFormat("typedef void (*cb)(int);", SpaceFuncDecl);
+  verifyFormat("T A::operator() ();", SpaceFuncDecl);
+  verifyFormat("X A::operator++ (T);", SpaceFuncDecl);
+  verifyFormat("T A::operator()() {}", SpaceFuncDecl);
+  verifyFormat("auto lambda = []() { return 0; };", SpaceFuncDecl);
+  verifyFormat("int x = int(y);", SpaceFuncDecl);
+  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SpaceFuncDecl);
+
+  FormatStyle SpaceFuncDef = getLLVMStyle();
+  SpaceFuncDef.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpaceFuncDef.SpaceBeforeParensOptions.AfterFunctionDefinitionName = true;
+
+  verifyFormat("int f();", SpaceFuncDef);
+  verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("#define A(x) x", SpaceFuncDef);
+  verifyFormat("#define A (x) x", SpaceFuncDef);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SpaceFuncDef);
+  verifyFormat("auto i = std::make_unique(5);", SpaceFuncDef);
+  verifyFormat("size_t x = sizeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> decltype(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> typeof(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> _Atomic(x);", SpaceFuncDef);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", SpaceFuncDef);
+  verifyFormat("int f(T x) noexcept(x.create());", SpaceFuncDef);
+  verifyFormat("alignas(128) char a[128];", SpaceFuncDef);
+  verifyFormat("size_t x = alignof(MyType);", 

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3170
+  // to add a space only before the first one
+  bool IsFirstLParen = true;
+  const FormatToken *Tok = Right.Previous;

I will simplify the following using Left.is(TT_FunctionDeclarationName)


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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/include/clang/Format/Format.h:3422
   ///true:  false:
-  ///for (auto v : values) {}   vs. for(auto v: values) {}
+  ///for (auto v : values) {}   vs. for (auto v: values) {}
   /// \endcode

HazardyKnusperkeks wrote:
> Please remove again. :)
I changed the documentation to have a space added or removed only before the 
for loop colon. Indeed the space after the for is handled by 
SpaceBeforeParens... and not SpaceBeforeRangeBasedForLoopColon. Why do you 
think it is better to keep it as before ?



Comment at: clang/lib/Format/Format.cpp:1221
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  LLVMStyle.SpaceBeforeParensFlags.AfterControlStatements = true;
+  LLVMStyle.SpaceBeforeParensFlags.AfterOperators = true;

HazardyKnusperkeks wrote:
> Is this needed? Shouldn't the expand take care of that?
Indeed the expand takes care of that and it would be nice not to have to repeat 
it here.
I had to add it for the test FormatTest.ConfigurationRoundTripTest, as it is 
done for LLVMStyle.BraceWrapping.
What do you think is best ?


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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Refactor SpaceBeforeParens to add flags

[Christian Rayroud via Phabricator via cfe-commits]

crayroud updated this revision to Diff 381168.
crayroud added a comment.

This new version adds the possibility to configure space before opening 
parentheses independently from one another. Instead of creating a new SBPO_ 
mode for each combinations.


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

https://reviews.llvm.org/D110833

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
@@ -14133,6 +14133,64 @@
   verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
+
+  FormatStyle SomeSpace2 = getLLVMStyle();
+  SomeSpace2.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SomeSpace2.SpaceBeforeParensFlags = {}; // Reset all flags to false
+  SomeSpace2.SpaceBeforeParensFlags.AfterControlStatements = true;
+  SomeSpace2.SpaceBeforeParensFlags.AfterFunctionDefinitionName = true;
+
+  verifyFormat("int f();", SomeSpace2);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace2);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace2);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace2);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace2);
+  verifyFormat("A::A () : a(1) {}", SomeSpace2);
+  verifyFormat("void f() __attribute__((asdf));", SomeSpace2);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace2);
+  verifyFormat("#define A(x) x", SomeSpace2);
+  verifyFormat("#define A (x) x", SomeSpace2);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace2);
+  verifyFormat("auto i = std::make_unique(5);", SomeSpace2);
+  verifyFormat("size_t x = sizeof(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> decltype(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> typeof(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> _Atomic(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", SomeSpace2);
+  verifyFormat("int f(T x) noexcept(x.create());", SomeSpace2);
+  verifyFormat("alignas(128) char a[128];", SomeSpace2);
+  verifyFormat("size_t x = alignof(MyType);", SomeSpace2);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SomeSpace2);
+  verifyFormat("int f() throw(Deprecated);", SomeSpace2);
+  verifyFormat("typedef void (*cb)(int);", SomeSpace2);
+  verifyFormat("T A::operator()();", SomeSpace2);
+  verifyFormat("X A::operator++(T);", SomeSpace2);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
+  verifyFormat("int x = int(y);", SomeSpace2);
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SomeSpace2);
 }
 
 TEST_F(FormatTest, SpaceAfterLogicalNot) {
@@ -18631,6 +18689,8 @@
   FormatStyle::SBPO_ControlStatementsExceptControlMacros);
   CHECK_PARSE("SpaceBeforeParens: NonEmptyParentheses", SpaceBeforeParens,
   FormatStyle::SBPO_NonEmptyParentheses);
+  CHECK_PARSE("SpaceBeforeParens: Custom", SpaceBeforeParens,
+  FormatStyle::SBPO_Custom);
   // For backward compatibility:
   CHECK_PARSE("SpaceAfterControlStatementKeyword: false", SpaceBeforeParens,
   FormatStyle::SBPO_Never);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2906,7 +2906,7 @@
 
 bool TokenAnnotator::spaceRequiredBeforeParens(const FormatToken ) const {
   return Style.SpaceBeforeParens == FormatStyle::SBPO_Always ||
- (Style.SpaceBeforeParens == FormatStyle::SBPO_NonEmptyParentheses &&
+ (Style.SpaceBeforeParensFlags.BeforeNonEmptyParentheses &&
   Right.ParameterCount > 0);
 }
 
@@ -3134,33 +3134,64 @@
   // e.g. template  [[nodiscard]] ...
   if (Left.is(TT_TemplateCloser) && Right.is(TT_AttributeSquare))
 return true;
+  // Space before parentheses common for all languages
   if (Right.is(tok::l_paren)) {
 if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
 (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
   return true;
-if (Style.SpaceBeforeParens ==
-

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added a comment.

Thank you for your inputs. I will work on the changes and update the patch once 
it is ready.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[Christian Rayroud via Phabricator via cfe-commits]

crayroud added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3649
 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for
 backward compatibility.
 

MyDeveloperDay wrote:
> Now I look back here, why where these Macro considered the same as for loops? 
> why would we want
> 
> ```
> for ()
> Q_FOREACH(...)
> ```
> 
> So this really does need a struct or we'll be eventually be adding
> 
> `SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacrosButNotIfAndDefinatelyWhilesAndSometimesSwitchButOnlyOnTheSecondThursdayOfTheMonth`
> 
> ```
> SpaceBeforeParen:
>  AfterFunctionDefinitionName: false
>  AfterFunctionDeclarationName: true
>  AfterSwitch: true
>  AfterForeachMacros: false
>   (there are likely others)
> ```
>  
> `
> 
> 
> 
> 
> 
Indeed replacing the enum with a struct as suggested is better to support the 
different possible combinations, compare to the current version of 
SpaceBeforeParens that results in very long names.

To support existing configuration files, I propose to keep the enum and to add 
a struct to handle the custom use cases and to cleanup the code. What do you 
think ?

```
SpaceBeforeParens: Custom
SpaceBeforeParensCustom:
 AfterFunctionDefinitionName: false
 AfterFunctionDeclarationName: true
 AfterSwitch: true
 AfterForeachMacros: false
…
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110833

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

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

[Christian Rayroud via Phabricator via cfe-commits]

crayroud created this revision.
crayroud added reviewers: aaron.ballman, rsmith.
crayroud added projects: clang, clang-format.
crayroud requested review of this revision.

For some projects the coding style defined requires to have a space before 
opening parentheses for function definitions.

This revision adds the 
ControlStatementsAndFunctionDefinitionsExceptControlMacros option to 
SpaceBeforeParens which act as SBPO_ControlStatementsExceptControlMacros but 
also put a space before opening parentheses for function definitions.

The goal of this commit is too add the support of clang-format to these 
projects.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D110833

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
@@ -2117,6 +2117,13 @@
"}",
Style);
 
+  Style.SpaceBeforeParens = FormatStyle::
+  SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacros;
+  verifyFormat("void f () {\n"
+   "  Q_FOREACH(Item *item, itemlist) {}\n"
+   "}",
+   Style);
+
   // As function-like macros.
   verifyFormat("#define foreach(x, y)\n"
"#define Q_FOREACH(x, y)\n"
@@ -14133,6 +14140,62 @@
   verifyFormat("X A::operator++ (T);", SomeSpace);
   verifyFormat("int x = int (y);", SomeSpace);
   verifyFormat("auto lambda = []() { return 0; };", SomeSpace);
+
+  FormatStyle SomeSpace2 = getLLVMStyle();
+  SomeSpace2.SpaceBeforeParens = FormatStyle::
+  SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacros;
+
+  verifyFormat("int f();", SomeSpace2);
+  verifyFormat("void f (int a, T b) {\n"
+   "  while (true)\n"
+   "continue;\n"
+   "}",
+   SomeSpace2);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   SomeSpace2);
+  verifyFormat("do {\n"
+   "  do_something();\n"
+   "} while (something());",
+   SomeSpace2);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   SomeSpace2);
+  verifyFormat("A::A () : a(1) {}", SomeSpace2);
+  verifyFormat("void f() __attribute__((asdf));", SomeSpace2);
+  verifyFormat("*( + 1);\n"
+   "&(()[1]);\n"
+   "a[(b + c) * d];\n"
+   "(((a + 1) * 2) + 3) * 4;",
+   SomeSpace2);
+  verifyFormat("#define A(x) x", SomeSpace2);
+  verifyFormat("#define A (x) x", SomeSpace2);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   SomeSpace2);
+  verifyFormat("auto i = std::make_unique(5);", SomeSpace2);
+  verifyFormat("size_t x = sizeof(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> decltype(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> typeof(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> _Atomic(x);", SomeSpace2);
+  verifyFormat("auto f(int x) -> __underlying_type(x);", SomeSpace2);
+  verifyFormat("int f(T x) noexcept(x.create());", SomeSpace2);
+  verifyFormat("alignas(128) char a[128];", SomeSpace2);
+  verifyFormat("size_t x = alignof(MyType);", SomeSpace2);
+  verifyFormat("static_assert(sizeof(char) == 1, \"Impossible!\");",
+   SomeSpace2);
+  verifyFormat("int f() throw(Deprecated);", SomeSpace2);
+  verifyFormat("typedef void (*cb)(int);", SomeSpace2);
+  verifyFormat("T A::operator()();", SomeSpace2);
+  verifyFormat("X A::operator++(T);", SomeSpace2);
+  verifyFormat("auto lambda = []() { return 0; };", SomeSpace2);
+  verifyFormat("int x = int(y);", SomeSpace2);
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
+   SomeSpace2);
 }
 
 TEST_F(FormatTest, SpaceAfterLogicalNot) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3138,12 +3138,18 @@
 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_ControlStatementsExceptControlMacros &&
+if ((Style.SpaceBeforeParens ==
+ FormatStyle::SBPO_ControlStatementsExceptControlMacros ||
+ Style.SpaceBeforeParens ==
+ FormatStyle::
+ SBPO_ControlStatementsAndFunctionDefinitionsExceptControlMacros) &&
 Left.is(TT_ForEachMacro))
   return false;
-if (Style.SpaceBeforeParens ==
-