[clang] [clang-format] Fix operator overload inconsistency in `BreakAfterAttributes: Always` (PR #74943)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 22 22:07:02 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/9] 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/9] 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/9] 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/9] 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/9] 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.
>From ddf4c777d8444559a19c3d0571369882f0768766 Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sun, 10 Dec 2023 00:25:47 -0300
Subject: [PATCH 6/9] Change test back to verifyFormat
---
clang/unittests/Format/FormatTest.cpp | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 00d735a3065d4b..9c79ed7389d703 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26454,12 +26454,17 @@ TEST_F(FormatTest, BreakAfterAttributes) {
Style.BreakBeforeBraces = FormatStyle::BS_Attach;
Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
- verifyNoChange("struct Foo {\n"
+ verifyFormat("struct Foo {\n"
" [[maybe_unused]]\n"
" void operator+();\n"
"};\n"
"[[nodiscard]]\n"
"Foo& operator-(Foo&);",
+
+ "struct Foo {\n"
+ "[[maybe_unused]] void operator+();\n"
+ "};\n"
+ "[[nodiscard]] Foo &operator-(Foo&);",
Style);
}
>From b3d39d984973f04ca94a32c3c3f5b2f5d29da49e Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sun, 10 Dec 2023 19:32:05 -0300
Subject: [PATCH 7/9] Addresses comments
---
clang/lib/Format/ContinuationIndenter.cpp | 21 ++++++++-------------
clang/unittests/Format/FormatTest.cpp | 21 ++++++++++-----------
2 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 645721667fbe57..9ff5994240a972 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -583,7 +583,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
return true;
}
- const auto WrapBeforeName = [&]() {
+ const auto WrapBeforeName = [&] {
// If the return type spans multiple lines, wrap before the function name.
if (Current.isNot(TT_FunctionDeclarationName) ||
State.Line->ReturnTypeWrapped) {
@@ -593,21 +593,16 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
return false;
if (!CurrentState.BreakBeforeParameter)
return false;
- return true;
+ // Don't break before a C# function when no break after return type.
+ return (!Style.isCSharp() ||
+ Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
+ // Don't always break between a JavaScript `function` and the
+ // function name.
+ !Style.isJavaScript();
};
- // Don't break before a C# function when no break after return type.
- if (WrapBeforeName() &&
- (!Style.isCSharp() ||
- Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None)) {
+ if (WrapBeforeName())
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.
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 9c79ed7389d703..1510ba51e69897 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26455,17 +26455,16 @@ TEST_F(FormatTest, BreakAfterAttributes) {
Style.BreakBeforeBraces = FormatStyle::BS_Attach;
Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
verifyFormat("struct Foo {\n"
- " [[maybe_unused]]\n"
- " void operator+();\n"
- "};\n"
- "[[nodiscard]]\n"
- "Foo& operator-(Foo&);",
-
- "struct Foo {\n"
- "[[maybe_unused]] void operator+();\n"
- "};\n"
- "[[nodiscard]] Foo &operator-(Foo&);",
- Style);
+ " [[maybe_unused]]\n"
+ " void operator+();\n"
+ "};\n"
+ "[[nodiscard]]\n"
+ "Foo& operator-(Foo&);",
+ "struct Foo {\n"
+ "[[maybe_unused]] void operator+();\n"
+ "};\n"
+ "[[nodiscard]] Foo &operator-(Foo&);",
+ Style);
}
TEST_F(FormatTest, InsertNewlineAtEOF) {
>From 58671eaaa96e39c74f75c511b295f284b45c978b Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Fri, 22 Dec 2023 09:57:55 -0300
Subject: [PATCH 8/9] Addresses owenca comment
---
clang/lib/Format/ContinuationIndenter.cpp | 30 +++++++++--------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 9ff5994240a972..e21e2acc9a69bf 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -583,26 +583,18 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
return true;
}
- 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;
- // Don't break before a C# function when no break after return type.
- return (!Style.isCSharp() ||
- Style.AlwaysBreakAfterReturnType != FormatStyle::RTBS_None) &&
- // Don't always break between a JavaScript `function` and the
- // function name.
- !Style.isJavaScript();
- };
-
- if (WrapBeforeName())
+ 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() && Previous.isNot(tok::kw_template) &&
+ CurrentState.BreakBeforeParameter) {
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 8528e75d84e35204de822ace491ab7f0a8a64272 Mon Sep 17 00:00:00 2001
From: XDeme <fernando.tagawa.gamail.com at gmail.com>
Date: Sat, 23 Dec 2023 03:06:46 -0300
Subject: [PATCH 9/9] Split tests
---
clang/unittests/Format/FormatTest.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 1510ba51e69897..1945daf005c827 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -26452,18 +26452,17 @@ TEST_F(FormatTest, BreakAfterAttributes) {
"}",
CtorDtorCode, Style);
- Style.BreakBeforeBraces = FormatStyle::BS_Attach;
- Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
verifyFormat("struct Foo {\n"
" [[maybe_unused]]\n"
" void operator+();\n"
"};\n"
"[[nodiscard]]\n"
+ "Foo &operator-(Foo &);",
+ Style);
+
+ Style.ReferenceAlignment = FormatStyle::ReferenceAlignmentStyle::RAS_Left;
+ verifyFormat("[[nodiscard]]\n"
"Foo& operator-(Foo&);",
- "struct Foo {\n"
- "[[maybe_unused]] void operator+();\n"
- "};\n"
- "[[nodiscard]] Foo &operator-(Foo&);",
Style);
}
More information about the cfe-commits
mailing list