[clang] [clang-format] Simplify the AfterPlacementOperator option (PR #79796)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 29 00:52:01 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: Owen Pan (owenca)
<details>
<summary>Changes</summary>
Change AfterPlacementOperator to a boolean. Also add SBPO_None for never inserting a space before a left parenthesis and deprecate SBPO_Never, which meant never inserting a space except when after new/delete.
Fixes #<!-- -->78892.
---
Full diff: https://github.com/llvm/llvm-project/pull/79796.diff
6 Files Affected:
- (modified) clang/docs/ClangFormatStyleOptions.rst (+12-25)
- (modified) clang/include/clang/Format/Format.h (+13-23)
- (modified) clang/lib/Format/Format.cpp (+6-22)
- (modified) clang/lib/Format/TokenAnnotator.cpp (+1-8)
- (modified) clang/unittests/Format/ConfigParseTest.cpp (+3-18)
- (modified) clang/unittests/Format/FormatTest.cpp (+14-18)
``````````diff
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 4dc0de3a90f2650..ce4bf867b26195f 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -5276,7 +5276,7 @@ the configuration (without a prefix: ``Auto``).
Possible values:
- * ``SBPO_Never`` (in configuration: ``Never``)
+ * ``SBPO_None`` (in configuration: ``None``)
Never put a space before opening parentheses.
.. code-block:: c++
@@ -5287,6 +5287,11 @@ the configuration (without a prefix: ``Auto``).
}
}
+ * ``SBPO_Never`` (in configuration: ``Never``)
+ This is **deprecated** and replaced by ``Custom`` below, with all
+ ``SpaceBeforeParensOptions`` but ``AfterPlacementOperator`` set to
+ ``false``.
+
* ``SBPO_ControlStatements`` (in configuration: ``ControlStatements``)
Put a space before opening parentheses only after control statement
keywords (``for/if/while...``).
@@ -5425,32 +5430,14 @@ the configuration (without a prefix: ``Auto``).
void operator++ (int a); vs. void operator++(int a);
object.operator++ (10); object.operator++(10);
- * ``AfterPlacementOperatorStyle AfterPlacementOperator`` :versionbadge:`clang-format 18`
-
- Defines in which cases to put a space between ``new/delete`` operators
- and opening parentheses.
+ * ``bool AfterPlacementOperator`` If ``true``, put a space between operator ``new``/``delete`` and opening
+ parenthesis.
- Possible values:
-
- * ``APO_Never`` (in configuration: ``Never``)
- Remove space after ``new/delete`` operators and before ``(``.
-
- .. code-block:: c++
-
- new(buf) T;
- delete(buf) T;
-
- * ``APO_Always`` (in configuration: ``Always``)
- Always add space after ``new/delete`` operators and before ``(``.
-
- .. code-block:: c++
-
- new (buf) T;
- delete (buf) T;
-
- * ``APO_Leave`` (in configuration: ``Leave``)
- Leave placement ``new/delete`` expressions as they are.
+ .. code-block:: c++
+ true: false:
+ new (buf) T; vs. new(buf) T;
+ delete (buf) T; delete(buf) T;
* ``bool AfterRequiresInClause`` If ``true``, put space between requires keyword in a requires clause and
opening parentheses, if there is one.
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index bc9eecd42f9ebfd..5c536bc3f381fab 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -4165,6 +4165,10 @@ struct FormatStyle {
/// }
/// }
/// \endcode
+ SBPO_None,
+ /// This is **deprecated** and replaced by ``Custom`` below, with all
+ /// ``SpaceBeforeParensOptions`` but ``AfterPlacementOperator`` set to
+ /// ``false``.
SBPO_Never,
/// Put a space before opening parentheses only after control statement
/// keywords (``for/if/while...``).
@@ -4273,28 +4277,14 @@ struct FormatStyle {
/// object.operator++ (10); object.operator++(10);
/// \endcode
bool AfterOverloadedOperator;
- /// Styles for adding spacing between ``new/delete`` operators and opening
- /// parentheses.
- enum AfterPlacementOperatorStyle : int8_t {
- /// Remove space after ``new/delete`` operators and before ``(``.
- /// \code
- /// new(buf) T;
- /// delete(buf) T;
- /// \endcode
- APO_Never,
- /// Always add space after ``new/delete`` operators and before ``(``.
- /// \code
- /// new (buf) T;
- /// delete (buf) T;
- /// \endcode
- APO_Always,
- /// Leave placement ``new/delete`` expressions as they are.
- APO_Leave,
- };
- /// Defines in which cases to put a space between ``new/delete`` operators
- /// and opening parentheses.
- /// \version 18
- AfterPlacementOperatorStyle AfterPlacementOperator;
+ /// If ``true``, put a space between operator ``new``/``delete`` and opening
+ /// parenthesis.
+ /// \code
+ /// true: false:
+ /// new (buf) T; vs. new(buf) T;
+ /// delete (buf) T; delete(buf) T;
+ /// \endcode
+ bool AfterPlacementOperator;
/// If ``true``, put space between requires keyword in a requires clause and
/// opening parentheses, if there is one.
/// \code
@@ -4327,7 +4317,7 @@ struct FormatStyle {
: AfterControlStatements(false), AfterForeachMacros(false),
AfterFunctionDeclarationName(false),
AfterFunctionDefinitionName(false), AfterIfMacros(false),
- AfterOverloadedOperator(false), AfterPlacementOperator(APO_Leave),
+ AfterOverloadedOperator(false), AfterPlacementOperator(true),
AfterRequiresInClause(false), AfterRequiresInExpression(false),
BeforeNonEmptyParentheses(false) {}
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index ff326dc784783b2..adf07b8225e7c65 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -504,22 +504,6 @@ struct ScalarEnumerationTraits<FormatStyle::QualifierAlignmentStyle> {
}
};
-template <>
-struct MappingTraits<
- FormatStyle::SpaceBeforeParensCustom::AfterPlacementOperatorStyle> {
- static void
- mapping(IO &IO,
- FormatStyle::SpaceBeforeParensCustom::AfterPlacementOperatorStyle
- &Value) {
- IO.enumCase(Value, "Always",
- FormatStyle::SpaceBeforeParensCustom::APO_Always);
- IO.enumCase(Value, "Never",
- FormatStyle::SpaceBeforeParensCustom::APO_Never);
- IO.enumCase(Value, "Leave",
- FormatStyle::SpaceBeforeParensCustom::APO_Leave);
- }
-};
-
template <> struct MappingTraits<FormatStyle::RawStringFormat> {
static void mapping(IO &IO, FormatStyle::RawStringFormat &Format) {
IO.mapOptional("Language", Format.Language);
@@ -707,6 +691,7 @@ template <> struct MappingTraits<FormatStyle::SpaceBeforeParensCustom> {
template <>
struct ScalarEnumerationTraits<FormatStyle::SpaceBeforeParensStyle> {
static void enumeration(IO &IO, FormatStyle::SpaceBeforeParensStyle &Value) {
+ IO.enumCase(Value, "None", FormatStyle::SBPO_None);
IO.enumCase(Value, "Never", FormatStyle::SBPO_Never);
IO.enumCase(Value, "ControlStatements",
FormatStyle::SBPO_ControlStatements);
@@ -1389,11 +1374,12 @@ static void expandPresetsSpaceBeforeParens(FormatStyle &Expanded) {
// Reset all flags
Expanded.SpaceBeforeParensOptions = {};
+ if (Expanded.SpaceBeforeParens == FormatStyle::SBPO_None)
+ return;
+
+ Expanded.SpaceBeforeParensOptions.AfterPlacementOperator = true;
+
switch (Expanded.SpaceBeforeParens) {
- case FormatStyle::SBPO_Never:
- Expanded.SpaceBeforeParensOptions.AfterPlacementOperator =
- FormatStyle::SpaceBeforeParensCustom::APO_Never;
- break;
case FormatStyle::SBPO_ControlStatements:
Expanded.SpaceBeforeParensOptions.AfterControlStatements = true;
Expanded.SpaceBeforeParensOptions.AfterForeachMacros = true;
@@ -1405,8 +1391,6 @@ static void expandPresetsSpaceBeforeParens(FormatStyle &Expanded) {
case FormatStyle::SBPO_NonEmptyParentheses:
Expanded.SpaceBeforeParensOptions.BeforeNonEmptyParentheses = true;
break;
- case FormatStyle::SBPO_Always:
- break;
default:
break;
}
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index df1c5bc19de1e84..d0c4273cfc7e583 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4274,14 +4274,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
Left.isOneOf(tok::kw_new, tok::kw_delete) &&
Right.isNot(TT_OverloadedOperatorLParen) &&
!(Line.MightBeFunctionDecl && Left.is(TT_FunctionDeclarationName))) {
- if (Style.SpaceBeforeParensOptions.AfterPlacementOperator ==
- FormatStyle::SpaceBeforeParensCustom::APO_Always ||
- (Style.SpaceBeforeParensOptions.AfterPlacementOperator ==
- FormatStyle::SpaceBeforeParensCustom::APO_Leave &&
- Right.hasWhitespaceBefore())) {
- return true;
- }
- return false;
+ return Style.SpaceBeforeParensOptions.AfterPlacementOperator;
}
if (Line.Type == LT_ObjCDecl)
return true;
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 2a8d79359a49b40..d92e373d24a871c 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -231,6 +231,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
AfterFunctionDefinitionName);
CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterIfMacros);
CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterOverloadedOperator);
+ CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, AfterPlacementOperator);
CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, BeforeNonEmptyParentheses);
CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InCStyleCasts);
CHECK_PARSE_NESTED_BOOL(SpacesInParensOptions, InConditionalStatements);
@@ -587,6 +588,8 @@ TEST(ConfigParseTest, ParsesConfiguration) {
SpaceAroundPointerQualifiers, FormatStyle::SAPQ_Both);
Style.SpaceBeforeParens = FormatStyle::SBPO_Always;
+ CHECK_PARSE("SpaceBeforeParens: None", SpaceBeforeParens,
+ FormatStyle::SBPO_None);
CHECK_PARSE("SpaceBeforeParens: Never", SpaceBeforeParens,
FormatStyle::SBPO_Never);
CHECK_PARSE("SpaceBeforeParens: Always", SpaceBeforeParens,
@@ -609,24 +612,6 @@ TEST(ConfigParseTest, ParsesConfiguration) {
SpaceBeforeParens,
FormatStyle::SBPO_ControlStatementsExceptControlMacros);
- Style.SpaceBeforeParens = FormatStyle::SBPO_Custom;
- Style.SpaceBeforeParensOptions.AfterPlacementOperator =
- FormatStyle::SpaceBeforeParensCustom::APO_Always;
- CHECK_PARSE("SpaceBeforeParensOptions:\n"
- " AfterPlacementOperator: Never",
- SpaceBeforeParensOptions.AfterPlacementOperator,
- FormatStyle::SpaceBeforeParensCustom::APO_Never);
-
- CHECK_PARSE("SpaceBeforeParensOptions:\n"
- " AfterPlacementOperator: Always",
- SpaceBeforeParensOptions.AfterPlacementOperator,
- FormatStyle::SpaceBeforeParensCustom::APO_Always);
-
- CHECK_PARSE("SpaceBeforeParensOptions:\n"
- " AfterPlacementOperator: Leave",
- SpaceBeforeParensOptions.AfterPlacementOperator,
- FormatStyle::SpaceBeforeParensCustom::APO_Leave);
-
// For backward compatibility:
Style.SpacesInParens = FormatStyle::SIPO_Never;
Style.SpacesInParensOptions = {};
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index e5e763edf5b5bf6..a471e36f8d6825f 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -11347,35 +11347,31 @@ TEST_F(FormatTest, UnderstandsNewAndDelete) {
FormatStyle AfterPlacementOperator = getLLVMStyle();
AfterPlacementOperator.SpaceBeforeParens = FormatStyle::SBPO_Custom;
- EXPECT_EQ(
- AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator,
- FormatStyle::SpaceBeforeParensCustom::APO_Leave);
+ EXPECT_TRUE(
+ AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator);
verifyFormat("new (buf) int;", AfterPlacementOperator);
- verifyFormat("new(buf) int;", AfterPlacementOperator);
-
- AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator =
- FormatStyle::SpaceBeforeParensCustom::APO_Never;
verifyFormat("struct A {\n"
" int *a;\n"
- " A(int *p) : a(new(p) int) {\n"
- " new(p) int;\n"
- " int *b = new(p) int;\n"
- " int *c = new(p) int(3);\n"
- " delete(b);\n"
+ " A(int *p) : a(new (p) int) {\n"
+ " new (p) int;\n"
+ " int *b = new (p) int;\n"
+ " int *c = new (p) int(3);\n"
+ " delete (b);\n"
" }\n"
"};",
AfterPlacementOperator);
verifyFormat("void operator new(void *foo) ATTRIB;", AfterPlacementOperator);
AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator =
- FormatStyle::SpaceBeforeParensCustom::APO_Always;
+ false;
+ verifyFormat("new(buf) int;", AfterPlacementOperator);
verifyFormat("struct A {\n"
" int *a;\n"
- " A(int *p) : a(new (p) int) {\n"
- " new (p) int;\n"
- " int *b = new (p) int;\n"
- " int *c = new (p) int(3);\n"
- " delete (b);\n"
+ " A(int *p) : a(new(p) int) {\n"
+ " new(p) int;\n"
+ " int *b = new(p) int;\n"
+ " int *c = new(p) int(3);\n"
+ " delete(b);\n"
" }\n"
"};",
AfterPlacementOperator);
``````````
</details>
https://github.com/llvm/llvm-project/pull/79796
More information about the cfe-commits
mailing list