[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-11-09 Thread Owen Pan via Phabricator via cfe-commits 
owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3970-4009
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren)) {
 if (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen))) {
   return Style.SpacesInParensOptions.InCStyleCasts;
 }
 if (Left.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||
 Right.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||

Consider wrapping this in a function or lambda. Not sure if it would simply the 
logic with the parens being handled separately:
```
if (Left.is(tok::l_paren)) {
  ...
} else if (Right.is(tok::r_paren)) {
  ...
}



Comment at: clang/lib/Format/TokenAnnotator.cpp:3990
+  return Style.SpacesInParensOptions.InFunctionDefinitions;
+else
+  return Style.SpacesInParensOptions.InFunctionDeclarations;

No `else` after `return`.



Comment at: clang/unittests/Format/FormatTest.cpp:16786
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);

gedare wrote:
> HazardyKnusperkeks wrote:
> > Why change this?
> The original test is incomplete/ambiguous. It's either a declaration missing 
> a semicolon, or it's the start of a definition. I made it a definition.
Then can you also add a declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-11-09 Thread Gedare Bloom via Phabricator via cfe-commits 
gedare added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-11-09 Thread Gedare Bloom via Phabricator via cfe-commits 
gedare updated this revision to Diff 558066.
gedare added a comment.

Rebase to D155529 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11064,7 +11064,8 @@
   verifyFormat("SomeType MemberFunction(const Deleted &) &;", Spaces);
 
   Spaces.SpacesInParensOptions.InCStyleCasts = false;
-  Spaces.SpacesInParensOptions.Other = true;
+  Spaces.SpacesInParensOptions.InFunctionDeclarations = true;
+  Spaces.SpacesInParensOptions.InOverloadedOperators = true;
   verifyFormat("Deleted =( const Deleted & ) & = default;", Spaces);
   verifyFormat("SomeType MemberFunction( const Deleted & ) & = delete;",
Spaces);
@@ -13631,6 +13632,7 @@
   SpaceBetweenBraces.SpacesInAngles = FormatStyle::SIAS_Always;
   SpaceBetweenBraces.SpacesInParens = FormatStyle::SIPO_Custom;
   SpaceBetweenBraces.SpacesInParensOptions.Other = true;
+  SpaceBetweenBraces.SpacesInParensOptions.InFunctionCalls = true;
   SpaceBetweenBraces.SpacesInSquareBrackets = true;
   verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
   verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
@@ -16685,6 +16687,7 @@
   Spaces.SpacesInParensOptions.Other = true;
   Spaces.SpacesInParensOptions.InConditionalStatements = true;
   Spaces.SpacesInParensOptions.InAttributeSpecifiers = true;
+  Spaces.SpacesInParensOptions.InFunctionCalls = true;
   verifyFormat("do_something( ::globalVar );", Spaces);
   verifyFormat("call( x, y, z );", Spaces);
   verifyFormat("call();", Spaces);
@@ -16712,7 +16715,7 @@
"}",
Spaces);
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);
 
   Spaces.SpacesInParens = FormatStyle::SIPO_Custom;
@@ -16727,6 +16730,142 @@
   verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
   verifyFormat("#define x (( int )-1)", Spaces);
 
+  // Run the first set of tests again with:
+  Spaces.SpacesInParens = FormatStyle::SIPO_Custom;
+  Spaces.SpacesInParensOptions = {};
+  Spaces.SpacesInParensOptions.InFunctionDeclarations = true;
+  verifyFormat("do_something(::globalVar);", Spaces);
+  verifyFormat("call(x, y, z);", Spaces);
+  verifyFormat("call();", Spaces);
+  verifyFormat("std::function callback;", Spaces);
+  verifyFormat("void inFunction() { std::function fct; }",
+   Spaces);
+  verifyFormat("while ((bool)1)\n"
+   "  continue;",
+   Spaces);
+  verifyFormat("for (;;)\n"
+   "  continue;",
+   Spaces);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   Spaces);
+  verifyFormat("do {\n"
+   "  do_something((int)i);\n"
+   "} while (something());",
+   Spaces);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   Spaces);
+  verifyFormat("SomeType *__attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("void __attribute__((naked)) foo( int bar );", Spaces);
+  verifyFormat("void f( int g ) __attribute__((asdf));", Spaces);
+  verifyFormat("int f();", Spaces);
+  verifyFormat("void f(int a, T b) {}", Spaces);
+  verifyFormat("void __attribute__((asdf)) f(int a, T b) {}", Spaces);
+  verifyFormat("A::A() : a(1) {}", Spaces);
+  verifyFormat("void f( int bar ) __attribute__((asdf));", Spaces);
+  verifyFormat("void __attribute__((asdf)) f( int bar );", Spaces);
+  verifyFormat("#define A(x) x", Spaces);
+  verifyFormat("#define A (x) x", Spaces);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   Spaces);
+  verifyFormat("auto i = std::make_unique(5);", Spaces);
+  verifyFormat("size_t x = sizeof(x);", Spaces);
+  verifyFormat("auto f( int x ) -> decltype(x);", Spaces);
+  verifyFormat("auto f( int x ) -> typeof(x);", Spaces);
+  verifyFormat("auto f( int x ) -> _Atomic(x);", Spaces);
+  verifyFormat("auto f( int x ) -> __underlying_type(x);", Spaces);
+  verifyFormat("int f( T x ) noexcept(x.create());", Spaces);
+  verifyFormat("alignas(128) char a[128];", Spaces);
+  verifyFormat("size_t x = alignof(MyType);", Spaces);
+

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-08-01 Thread Gedare Bloom via Phabricator via cfe-commits 
gedare added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:16786
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);

HazardyKnusperkeks wrote:
> Why change this?
The original test is incomplete/ambiguous. It's either a declaration missing a 
semicolon, or it's the start of a definition. I made it a definition.



Comment at: clang/unittests/Format/FormatTest.cpp:16904
+  verifyFormat("call( x, y, z );", Spaces);
+  verifyFormat("call();", Spaces);
+  verifyFormat("std::function callback;", Spaces);

HazardyKnusperkeks wrote:
> Should here be a space?
That's controlled by space in empty parens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-07-27 Thread Björn Schäpers via Phabricator via cfe-commits 
HazardyKnusperkeks added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:16786
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);

Why change this?



Comment at: clang/unittests/Format/FormatTest.cpp:16904
+  verifyFormat("call( x, y, z );", Spaces);
+  verifyFormat("call();", Spaces);
+  verifyFormat("std::function callback;", Spaces);

Should here be a space?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-07-26 Thread Gedare Bloom via Phabricator via cfe-commits 
gedare updated this revision to Diff 544544.
gedare added a comment.

Add tests and correct function call detection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11040,7 +11040,8 @@
   verifyFormat("SomeType MemberFunction(const Deleted &) &;", Spaces);
 
   Spaces.SpacesInParensOptions.InCStyleCasts = false;
-  Spaces.SpacesInParensOptions.Other = true;
+  Spaces.SpacesInParensOptions.InFunctionDeclarations = true;
+  Spaces.SpacesInParensOptions.InOverloadedOperators = true;
   verifyFormat("Deleted =( const Deleted & ) & = default;", Spaces);
   verifyFormat("SomeType MemberFunction( const Deleted & ) & = delete;",
Spaces);
@@ -13678,6 +13679,7 @@
   SpaceBetweenBraces.SpacesInAngles = FormatStyle::SIAS_Always;
   SpaceBetweenBraces.SpacesInParens = FormatStyle::SIPO_Custom;
   SpaceBetweenBraces.SpacesInParensOptions.Other = true;
+  SpaceBetweenBraces.SpacesInParensOptions.InFunctionCalls = true;
   SpaceBetweenBraces.SpacesInSquareBrackets = true;
   verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
   verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
@@ -16753,6 +16755,7 @@
   Spaces.SpacesInParensOptions.Other = true;
   Spaces.SpacesInParensOptions.InConditionalStatements = true;
   Spaces.SpacesInParensOptions.InAttributeSpecifiers = true;
+  Spaces.SpacesInParensOptions.InFunctionCalls = true;
   verifyFormat("do_something( ::globalVar );", Spaces);
   verifyFormat("call( x, y, z );", Spaces);
   verifyFormat("call();", Spaces);
@@ -16780,7 +16783,7 @@
"}",
Spaces);
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);
 
   Spaces.SpacesInParens = FormatStyle::SIPO_Custom;
@@ -16795,6 +16798,142 @@
   verifyFormat("my_int a = ( my_int )sizeof(int);", Spaces);
   verifyFormat("#define x (( int )-1)", Spaces);
 
+  // Run the first set of tests again with:
+  Spaces.SpacesInParens = FormatStyle::SIPO_Custom;
+  Spaces.SpacesInParensOptions = {};
+  Spaces.SpacesInParensOptions.InFunctionDeclarations = true;
+  verifyFormat("do_something(::globalVar);", Spaces);
+  verifyFormat("call(x, y, z);", Spaces);
+  verifyFormat("call();", Spaces);
+  verifyFormat("std::function callback;", Spaces);
+  verifyFormat("void inFunction() { std::function fct; }",
+   Spaces);
+  verifyFormat("while ((bool)1)\n"
+   "  continue;",
+   Spaces);
+  verifyFormat("for (;;)\n"
+   "  continue;",
+   Spaces);
+  verifyFormat("if (true)\n"
+   "  f();\n"
+   "else if (true)\n"
+   "  f();",
+   Spaces);
+  verifyFormat("do {\n"
+   "  do_something((int)i);\n"
+   "} while (something());",
+   Spaces);
+  verifyFormat("switch (x) {\n"
+   "default:\n"
+   "  break;\n"
+   "}",
+   Spaces);
+  verifyFormat("SomeType *__attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("void __attribute__((naked)) foo( int bar );", Spaces);
+  verifyFormat("void f( int g ) __attribute__((asdf));", Spaces);
+  verifyFormat("int f();", Spaces);
+  verifyFormat("void f(int a, T b) {}", Spaces);
+  verifyFormat("void __attribute__((asdf)) f(int a, T b) {}", Spaces);
+  verifyFormat("A::A() : a(1) {}", Spaces);
+  verifyFormat("void f( int bar ) __attribute__((asdf));", Spaces);
+  verifyFormat("void __attribute__((asdf)) f( int bar );", Spaces);
+  verifyFormat("#define A(x) x", Spaces);
+  verifyFormat("#define A (x) x", Spaces);
+  verifyFormat("#if defined(x)\n"
+   "#endif",
+   Spaces);
+  verifyFormat("auto i = std::make_unique(5);", Spaces);
+  verifyFormat("size_t x = sizeof(x);", Spaces);
+  verifyFormat("auto f( int x ) -> decltype(x);", Spaces);
+  verifyFormat("auto f( int x ) -> typeof(x);", Spaces);
+  verifyFormat("auto f( int x ) -> _Atomic(x);", Spaces);
+  verifyFormat("auto f( int x ) -> __underlying_type(x);", Spaces);
+  verifyFormat("int f( T x ) noexcept(x.create());", Spaces);
+  verifyFormat("alignas(128) char a[128];", Spaces);
+  verifyFormat("size_t x = alignof(MyType);", Spaces);
+

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-07-26 Thread Björn Schäpers via Phabricator via cfe-commits 
HazardyKnusperkeks added a comment.

You need some tests to show that the new options really apply like intended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-07-26 Thread Gedare Bloom via Phabricator via cfe-commits 
gedare created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
gedare requested review of this revision.

This change separates function calls, declarations, definitions, and
overloaded operators from `SpacesInParensOptions.Other` to allow control
over each independently.

Fixes Github Issue \#55428.

Depends on D155529 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156360

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/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11040,7 +11040,8 @@
   verifyFormat("SomeType MemberFunction(const Deleted &) &;", Spaces);
 
   Spaces.SpacesInParensOptions.InCStyleCasts = false;
-  Spaces.SpacesInParensOptions.Other = true;
+  Spaces.SpacesInParensOptions.InFunctionDeclarations = true;
+  Spaces.SpacesInParensOptions.InOverloadedOperators = true;
   verifyFormat("Deleted =( const Deleted & ) & = default;", Spaces);
   verifyFormat("SomeType MemberFunction( const Deleted & ) & = delete;",
Spaces);
@@ -13678,6 +13679,7 @@
   SpaceBetweenBraces.SpacesInAngles = FormatStyle::SIAS_Always;
   SpaceBetweenBraces.SpacesInParens = FormatStyle::SIPO_Custom;
   SpaceBetweenBraces.SpacesInParensOptions.Other = true;
+  SpaceBetweenBraces.SpacesInParensOptions.InFunctionCalls = true;
   SpaceBetweenBraces.SpacesInSquareBrackets = true;
   verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
   verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
@@ -16749,6 +16751,7 @@
   Spaces.SpacesInParensOptions.Other = true;
   Spaces.SpacesInParensOptions.InConditionalStatements = true;
   Spaces.SpacesInParensOptions.InAttributeSpecifiers = true;
+  Spaces.SpacesInParensOptions.InFunctionCalls = true;
   verifyFormat("do_something( ::globalVar );", Spaces);
   verifyFormat("call( x, y, z );", Spaces);
   verifyFormat("call();", Spaces);
@@ -23819,6 +23822,7 @@
   Style.SpacesInParensOptions.InCStyleCasts = true;
   verifyFormat("x = ( _Atomic(uint64_t) )*a;", Style);
   Style.SpacesInParensOptions.InCStyleCasts = false;
+  Style.SpacesInParensOptions.InFunctionCalls = true;
   Style.SpacesInParensOptions.Other = true;
   verifyFormat("x = (_Atomic( uint64_t ))*a;", Style);
   verifyFormat("x = (_Atomic( uint64_t ))", Style);
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -226,6 +226,10 @@
   CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InCStyleCasts);
   CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InConditionalStatements);
   CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InEmptyParentheses);
+  CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InFunctionCalls);
+  CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InFunctionDeclarations);
+  CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InFunctionDefinitions);
+  CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InOverloadedOperators);
   CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, Other);
 }
 
@@ -600,23 +604,23 @@
   Style.SpacesInParens = FormatStyle::SIPO_Never;
   Style.SpacesInParensOptions = {};
   CHECK_PARSE("SpacesInParentheses: true", SpacesInParensOptions,
-  FormatStyle::SpacesInParensCustom(true, true, false, false,
-  true));
+  FormatStyle::SpacesInParensCustom(true, true, false, false, true,
+true, true, true, true));
   Style.SpacesInParens = FormatStyle::SIPO_Never;
   Style.SpacesInParensOptions = {};
   CHECK_PARSE("SpacesInConditionalStatement: true", SpacesInParensOptions,
   FormatStyle::SpacesInParensCustom(false, true, false, false,
-  false));
+false, false, false, false, false));
   Style.SpacesInParens = FormatStyle::SIPO_Never;
   Style.SpacesInParensOptions = {};
   CHECK_PARSE("SpacesInCStyleCastParentheses: true", SpacesInParensOptions,
   FormatStyle::SpacesInParensCustom(false, false, true, false,
-  false));
+false, false, false, false, false));
   Style.SpacesInParens = FormatStyle::SIPO_Never;
   Style.SpacesInParensOptions = {};
   CHECK_PARSE("SpaceInEmptyParentheses: true", SpacesInParensOptions,
   FormatStyle::SpacesInParensCustom(false, false, false, true,
-  false));
+false, false, false, false, false));
   Style.SpacesInParens =