[clang] [clang-format] Fix operator overload inconsistency in `BreakAfterAttributes: Always` (PR #74943)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 9 15:56:58 PST 2023
https://github.com/XDeme updated https://github.com/llvm/llvm-project/pull/74943
>From b80f8579dbc745ddfaa3d60770dd0d3e79e6c641 Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sat, 9 Dec 2023 14:31:12 -0300
Subject: [PATCH 1/5] Fixes overload operator in BreakAfterAttributes
---
clang/lib/Format/ContinuationIndenter.cpp | 3 ++-
clang/unittests/Format/FormatTest.cpp | 14 ++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 9e4e939503dfe4..de3768d475e7b2 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -593,7 +593,8 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
// name.
!Style.isJavaScript()) ||
(Current.is(tok::kw_operator) && Previous.isNot(tok::coloncolon))) &&
- Previous.isNot(tok::kw_template) && CurrentState.BreakBeforeParameter) {
+ Previous.isNot(tok::kw_template) && CurrentState.BreakBeforeParameter &&
+ (Style.isCpp() && Current.Tok.isNot(tok::kw_operator))) {
return true;
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 24b2fd599dc397..a1f3beed475ff3 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26417,6 +26417,20 @@ TEST_F(FormatTest, BreakAfterAttributes) {
"void g() {}",
CtorDtorCode, Style);
+ Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
+ constexpr StringRef OperatorOverloadCode(
+ "struct Foo {\n"
+ "[[maybe_unused]] void operator+();\n"
+ "};\n"
+ "[[nodiscard]] Foo& operator-(Foo&);");
+ verifyFormat("struct Foo {\n"
+ " [[maybe_unused]]\n"
+ " void operator+();\n"
+ "};\n"
+ "[[nodiscard]]\n"
+ "Foo& operator-(Foo&);",
+ OperatorOverloadCode, Style);
+
Style.BreakBeforeBraces = FormatStyle::BS_Linux;
verifyFormat("struct Foo {\n"
" [[deprecated]]\n"
>From 67a018f8c27f547fdea3443100ec7255e7aa4f2e Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sat, 9 Dec 2023 16:05:12 -0300
Subject: [PATCH 2/5] Addresses 1, 2 and 4 comments
---
clang/lib/Format/ContinuationIndenter.cpp | 2 +-
clang/unittests/Format/FormatTest.cpp | 20 ++++++++------------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index de3768d475e7b2..d05a16f87038ce 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -594,7 +594,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
!Style.isJavaScript()) ||
(Current.is(tok::kw_operator) && Previous.isNot(tok::coloncolon))) &&
Previous.isNot(tok::kw_template) && CurrentState.BreakBeforeParameter &&
- (Style.isCpp() && Current.Tok.isNot(tok::kw_operator))) {
+ Current.Tok.isNot(tok::kw_operator)) {
return true;
}
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index a1f3beed475ff3..bccc3162c34367 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26418,18 +26418,14 @@ TEST_F(FormatTest, BreakAfterAttributes) {
CtorDtorCode, Style);
Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
- constexpr StringRef OperatorOverloadCode(
- "struct Foo {\n"
- "[[maybe_unused]] void operator+();\n"
- "};\n"
- "[[nodiscard]] Foo& operator-(Foo&);");
- verifyFormat("struct Foo {\n"
- " [[maybe_unused]]\n"
- " void operator+();\n"
- "};\n"
- "[[nodiscard]]\n"
- "Foo& operator-(Foo&);",
- OperatorOverloadCode, Style);
+ verifyNoChange("struct Foo {\n"
+ " [[maybe_unused]]\n"
+ " void operator+();\n"
+ "};\n"
+ "[[nodiscard]]\n"
+ "Foo& operator-(Foo&);",
+ Style);
+ Style.ReferenceAlignment = getLLVMStyle().ReferenceAlignment;
Style.BreakBeforeBraces = FormatStyle::BS_Linux;
verifyFormat("struct Foo {\n"
>From f9b8d8cee9cf1fd313497c72e7829114a5d4b083 Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sat, 9 Dec 2023 18:46:15 -0300
Subject: [PATCH 3/5] Addresses comment 3
---
clang/lib/Format/ContinuationIndenter.cpp | 25 +++++++++++++----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index d05a16f87038ce..a9ce3d20142984 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -584,20 +584,23 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
}
// If the return type spans multiple lines, wrap before the function name.
- if (((Current.is(TT_FunctionDeclarationName) &&
- !State.Line->ReturnTypeWrapped &&
- // Don't break before a C# function when no break after return type.
- (!Style.isCSharp() ||
- Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
- // Don't always break between a JavaScript `function` and the function
- // name.
- !Style.isJavaScript()) ||
- (Current.is(tok::kw_operator) && Previous.isNot(tok::coloncolon))) &&
- Previous.isNot(tok::kw_template) && CurrentState.BreakBeforeParameter &&
- Current.Tok.isNot(tok::kw_operator)) {
+ const bool WrapBeforeName =
+ Current.is(TT_FunctionDeclarationName) &&
+ !State.Line->ReturnTypeWrapped && Previous.isNot(tok::kw_template) &&
+ Current.isNot(tok::kw_operator) && CurrentState.BreakBeforeParameter;
+
+ // Don't break before a C# function when no break after return type.
+ if (WrapBeforeName &&
+ (!Style.isCSharp() ||
+ Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None)) {
return true;
}
+ // Don't always break between a JavaScript `function` and the function
+ // name.
+ if (WrapBeforeName && !Style.isJavaScript())
+ return true;
+
// The following could be precomputed as they do not depend on the state.
// However, as they should take effect only if the UnwrappedLine does not fit
// into the ColumnLimit, they are checked here in the ContinuationIndenter.
>From 6158f2b6d592debffe1c72beb96e5a99b377ff76 Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sat, 9 Dec 2023 18:59:02 -0300
Subject: [PATCH 4/5] Move test to the bottom
---
clang/unittests/Format/FormatTest.cpp | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index bccc3162c34367..00d735a3065d4b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26417,16 +26417,6 @@ TEST_F(FormatTest, BreakAfterAttributes) {
"void g() {}",
CtorDtorCode, Style);
- Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
- verifyNoChange("struct Foo {\n"
- " [[maybe_unused]]\n"
- " void operator+();\n"
- "};\n"
- "[[nodiscard]]\n"
- "Foo& operator-(Foo&);",
- Style);
- Style.ReferenceAlignment = getLLVMStyle().ReferenceAlignment;
-
Style.BreakBeforeBraces = FormatStyle::BS_Linux;
verifyFormat("struct Foo {\n"
" [[deprecated]]\n"
@@ -26461,6 +26451,16 @@ TEST_F(FormatTest, BreakAfterAttributes) {
"{\n"
"}",
CtorDtorCode, Style);
+
+ Style.BreakBeforeBraces = FormatStyle::BS_Attach;
+ Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
+ verifyNoChange("struct Foo {\n"
+ " [[maybe_unused]]\n"
+ " void operator+();\n"
+ "};\n"
+ "[[nodiscard]]\n"
+ "Foo& operator-(Foo&);",
+ Style);
}
TEST_F(FormatTest, InsertNewlineAtEOF) {
>From f63b3a7386003016e693352ea198dcd14d36787a Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sat, 9 Dec 2023 20:56:15 -0300
Subject: [PATCH 5/5] Make use of a lambda and early return false
---
clang/lib/Format/ContinuationIndenter.cpp | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index a9ce3d20142984..645721667fbe57 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -583,14 +583,21 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
return true;
}
- // If the return type spans multiple lines, wrap before the function name.
- const bool WrapBeforeName =
- Current.is(TT_FunctionDeclarationName) &&
- !State.Line->ReturnTypeWrapped && Previous.isNot(tok::kw_template) &&
- Current.isNot(tok::kw_operator) && CurrentState.BreakBeforeParameter;
+ const auto WrapBeforeName = [&]() {
+ // If the return type spans multiple lines, wrap before the function name.
+ if (Current.isNot(TT_FunctionDeclarationName) ||
+ State.Line->ReturnTypeWrapped) {
+ return false;
+ }
+ if (Previous.is(tok::kw_template) || Current.is(tok::kw_operator))
+ return false;
+ if (!CurrentState.BreakBeforeParameter)
+ return false;
+ return true;
+ };
// Don't break before a C# function when no break after return type.
- if (WrapBeforeName &&
+ if (WrapBeforeName() &&
(!Style.isCSharp() ||
Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None)) {
return true;
@@ -598,7 +605,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
// Don't always break between a JavaScript `function` and the function
// name.
- if (WrapBeforeName && !Style.isJavaScript())
+ if (WrapBeforeName() && !Style.isJavaScript())
return true;
// The following could be precomputed as they do not depend on the state.
More information about the cfe-commits
mailing list