[clang] [clang-format] add an option to insert a space only for non-code block empty braces (PR #93634)
via cfe-commits
cfe-commits at lists.llvm.org
Tue May 28 20:07:27 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Kohei Asano (khei4)
<details>
<summary>Changes</summary>
# What
This patch introduces an option to insert a space between empty braces, especially for empty initializer.
# Motivation
WebKit has [its own `.clang-format` ](https://github.com/WebKit/WebKit/blob/main/.clang-format) and [clang-format has `basedOnStyle: WebKit`](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#basedonstyle), but it's inconsistent with [internal style checker script](https://github.com/WebKit/WebKit/blob/main/Tools/Scripts/check-webkit-style), at least for the empty initializers.
For example, the following line, tweaked from [WebKit source code](https://github.com/WebKit/WebKit/blob/main/Source/WebKit/UIProcess/Inspector/win/WebInspectorUIProxyWin.cpp#L222), would be checked by the script
```c++
toImpl(listenerRef)->use( {} );
```
```bash
$ python Tools/Scripts/check-webkit-style
ERROR: Source/WebKit/UIProcess/Inspector/win/WebInspectorUIProxyWin.cpp:222: Extra space after ( in function call [whitespace/parens] [4]
ERROR: Source/WebKit/UIProcess/Inspector/win/WebInspectorUIProxyWin.cpp:222: Extra space before ) [whitespace/parens] [2]
ERROR: Source/WebKit/UIProcess/Inspector/win/WebInspectorUIProxyWin.cpp:222: Missing space inside { }. [whitespace/braces] [5]
```
A space in empty braces is required, but one in (also for empty) parens isn't allowed.
Current clang-format could insert a space in empty parenthesis and block simultaneously by turning on [SpacesInParensOptions.InEmptyParentheses](https://clang.llvm.org/docs/ClangFormatStyleOptions.html#spacesinparensoptions), but formatting only braces couldn't be achieved.
So we want to insert a space only for braces.
# Note
I guess compatibility is important, but including braces in parens options might have been miss-leading. If more descriptive option name and desirable changes exist, I'm very happy to hear that :)
---
Full diff: https://github.com/llvm/llvm-project/pull/93634.diff
6 Files Affected:
- (modified) clang/docs/ClangFormatStyleOptions.rst (+13-1)
- (modified) clang/include/clang/Format/Format.h (+17-4)
- (modified) clang/lib/Format/Format.cpp (+1)
- (modified) clang/lib/Format/TokenAnnotator.cpp (+5)
- (modified) clang/unittests/Format/ConfigParseTest.cpp (+13-8)
- (modified) clang/unittests/Format/FormatTest.cpp (+5)
``````````diff
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 6d092219877f9..a1944eec8582b 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -6237,18 +6237,30 @@ the configuration (without a prefix: ``Auto``).
true: false:
x = ( int32 )y vs. x = (int32)y
- * ``bool InEmptyParentheses`` Put a space in parentheses only if the parentheses are empty i.e. '()'
+ * ``bool InEmptyParentheses`` Put a space in parentheses and braces only if they are empty i.e. '()' or '{}'
.. code-block:: c++
true: false:
void f( ) { vs. void f() {
int x[] = {foo( ), bar( )}; int x[] = {foo(), bar()};
+ T a = { }; T a = {};
if (true) { if (true) {
f( ); f();
} }
} }
+ * ``bool InEmptyBraces`` Put a space in *only* braces, not for parentheses, only if the braces are empty i.e. '{}'
+
+ .. code-block:: c++
+
+ true: false:
+ void f() { vs. void f() {
+ T x = {}; T x = { };
+ g(x, {}); g(x, { });
+ } }
+
+
* ``bool Other`` Put a space in parentheses not covered by preceding options.
.. code-block:: c++
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 274b45d1bc586..4482f7fb49788 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -4673,6 +4673,17 @@ struct FormatStyle {
/// } }
/// \endcode
bool InEmptyParentheses;
+ /// Put a space in brackets only if the parentheses are empty i.e. '()'
+ /// \code
+ /// true: false:
+ /// void f( ) { vs. void f() {
+ /// int x[] = {foo( ), bar( )}; int x[] = {foo(), bar()};
+ /// if (true) { if (true) {
+ /// f( ); f();
+ /// } }
+ /// } }
+ /// \endcode
+ bool InEmptyBraces;
/// Put a space in parentheses not covered by preceding options.
/// \code
/// true: false:
@@ -4682,18 +4693,20 @@ struct FormatStyle {
SpacesInParensCustom()
: InConditionalStatements(false), InCStyleCasts(false),
- InEmptyParentheses(false), Other(false) {}
+ InEmptyParentheses(false), InEmptyBraces(false), Other(false) {}
SpacesInParensCustom(bool InConditionalStatements, bool InCStyleCasts,
- bool InEmptyParentheses, bool Other)
+ bool InEmptyParentheses, bool InEmptyBraces,
+ bool Other)
: InConditionalStatements(InConditionalStatements),
InCStyleCasts(InCStyleCasts), InEmptyParentheses(InEmptyParentheses),
- Other(Other) {}
+ InEmptyBraces(InEmptyBraces), Other(Other) {}
bool operator==(const SpacesInParensCustom &R) const {
return InConditionalStatements == R.InConditionalStatements &&
InCStyleCasts == R.InCStyleCasts &&
- InEmptyParentheses == R.InEmptyParentheses && Other == R.Other;
+ InEmptyParentheses == R.InEmptyParentheses &&
+ InEmptyBraces == R.InEmptyBraces && Other == R.Other;
}
bool operator!=(const SpacesInParensCustom &R) const {
return !(*this == R);
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 9cba0c2614eef..efcc48b910718 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -723,6 +723,7 @@ template <> struct MappingTraits<FormatStyle::SpacesInParensCustom> {
IO.mapOptional("InCStyleCasts", Spaces.InCStyleCasts);
IO.mapOptional("InConditionalStatements", Spaces.InConditionalStatements);
IO.mapOptional("InEmptyParentheses", Spaces.InEmptyParentheses);
+ IO.mapOptional("InEmptyBraces", Spaces.InEmptyBraces);
IO.mapOptional("Other", Spaces.Other);
}
};
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 7c4c76a91f2c5..094c182c8d448 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4341,6 +4341,11 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
Right.MatchingParen == &Left && Line.Children.empty()) {
return Style.SpaceInEmptyBlock;
}
+ if (Style.SpacesInParensOptions.InEmptyBraces &&
+ (Left.is(tok::l_brace) && Left.isNot(BK_Block) &&
+ Right.is(tok::r_brace) && Right.isNot(BK_Block))) {
+ return Style.SpacesInParensOptions.InEmptyBraces;
+ }
if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
(Left.is(tok::l_brace) && Left.isNot(BK_Block) &&
Right.is(tok::r_brace) && Right.isNot(BK_Block))) {
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 82e72f08ffb5e..c8918d4cec128 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -238,6 +238,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InCStyleCasts);
CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InConditionalStatements);
CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InEmptyParentheses);
+ CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InEmptyBraces);
CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, Other);
}
@@ -619,20 +620,24 @@ TEST(ConfigParseTest, ParsesConfiguration) {
FormatStyle::SIPO_Custom);
Style.SpacesInParens = FormatStyle::SIPO_Never;
Style.SpacesInParensOptions = {};
- CHECK_PARSE("SpacesInParentheses: true", SpacesInParensOptions,
- FormatStyle::SpacesInParensCustom(true, false, false, true));
+ CHECK_PARSE(
+ "SpacesInParentheses: true", SpacesInParensOptions,
+ FormatStyle::SpacesInParensCustom(true, false, false, false, true));
Style.SpacesInParens = FormatStyle::SIPO_Never;
Style.SpacesInParensOptions = {};
- CHECK_PARSE("SpacesInConditionalStatement: true", SpacesInParensOptions,
- FormatStyle::SpacesInParensCustom(true, false, false, false));
+ CHECK_PARSE(
+ "SpacesInConditionalStatement: true", SpacesInParensOptions,
+ FormatStyle::SpacesInParensCustom(true, false, false, false, false));
Style.SpacesInParens = FormatStyle::SIPO_Never;
Style.SpacesInParensOptions = {};
- CHECK_PARSE("SpacesInCStyleCastParentheses: true", SpacesInParensOptions,
- FormatStyle::SpacesInParensCustom(false, true, false, false));
+ CHECK_PARSE(
+ "SpacesInCStyleCastParentheses: true", SpacesInParensOptions,
+ FormatStyle::SpacesInParensCustom(false, true, false, false, false));
Style.SpacesInParens = FormatStyle::SIPO_Never;
Style.SpacesInParensOptions = {};
- CHECK_PARSE("SpaceInEmptyParentheses: true", SpacesInParensOptions,
- FormatStyle::SpacesInParensCustom(false, false, true, false));
+ CHECK_PARSE(
+ "SpaceInEmptyParentheses: true", SpacesInParensOptions,
+ FormatStyle::SpacesInParensCustom(false, false, true, false, false));
Style.SpacesInParens = FormatStyle::SIPO_Never;
Style.SpacesInParensOptions = {};
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 2f0c0f0266774..f154941426f2c 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -14027,6 +14027,11 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
SpaceBetweenBraces.SpacesInParens = FormatStyle::SIPO_Custom;
SpaceBetweenBraces.SpacesInParensOptions.InEmptyParentheses = true;
verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
+ SpaceBetweenBraces.SpacesInParens = FormatStyle::SIPO_Custom;
+ SpaceBetweenBraces.SpacesInParensOptions.InEmptyParentheses = false;
+ SpaceBetweenBraces.SpacesInParensOptions.Other = false;
+ SpaceBetweenBraces.SpacesInParensOptions.InEmptyBraces = true;
+ verifyFormat("T x = { };\nf(x, { });\ng();", SpaceBetweenBraces);
}
TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/93634
More information about the cfe-commits
mailing list