[clang-format] revert to string << string handling to previous default #88490
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
default Fixes 88433 A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviour behind a new option.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-format @llvm/pr-subscribers-clang Author: MyDeveloperDay (mydeveloperday) ChangesFixes 88433 A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviour behind a new option. Full diff: https://github.com/llvm/llvm-project/pull/88490.diff 7 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 39f7cded36edbf..a40a940f39d860 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3258,6 +3258,40 @@ the configuration (without a prefix: ``Auto``).
firstValue :
SecondValueVeryVeryVeryVeryLong;
+.. _BreakChevronOperator:
+
+**BreakChevronOperator** (``BreakChevronOperatorStyle``) :versionbadge:`clang-format 19` :ref:`¶ <BreakChevronOperator>`
+ Break Between Chevron Operators
+
+ Possible values:
+
+ * ``BCOS_Never`` (in configuration: ``Never``)
+ Break using ColumnLimit rules.
+
+ .. code-block:: c++
+
+ os << "aaaaa" << "bbbbb" << "\n";
+
+ * ``BCOS_BetweenStrings`` (in configuration: ``BetweenStrings``)
+ Break between adjacent strings.
+
+ .. code-block:: c++
+
+ os << "aaaaa"
+ << "bbbbb"
+ << "\n";
+
+ * ``BCOS_BetweenNewlineStrings`` (in configuration: ``BetweenNewlineStrings``)
+ Break between adjacent strings that end with \n.
+
+ .. code-block:: c++
+
+ os << "aaaaa\n"
+ << "bbbbb" << "ccccc\n"
+ << "\n";
+
+
+
.. _BreakConstructorInitializers:
**BreakConstructorInitializers** (``BreakConstructorInitializersStyle``) :versionbadge:`clang-format 5` :ref:`¶ <BreakConstructorInitializers>`
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 45a9a79739a4eb..01838b0ccd653d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -671,6 +671,8 @@ clang-format
``BreakTemplateDeclarations``.
- ``AlwaysBreakAfterReturnType`` is deprecated and renamed to
``BreakAfterReturnType``.
+- ``BreakChevronOperator`` Style is added and the previous default
+ of breaking between strings is reverted.
libclang
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 48f5fb44157570..205c597af8fb0f 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2193,6 +2193,33 @@ struct FormatStyle {
/// \version 3.7
bool BreakBeforeTernaryOperators;
+ /// Different ways to Break Between Chevrons
+ enum BreakChevronOperatorStyle : int8_t {
+ /// Break using ColumnLimit rules.
+ /// \code
+ /// os << "aaaaa" << "bbbbb" << "\n";
+ /// \endcode
+ BCOS_Never,
+ /// Break between adjacent strings.
+ /// \code
+ /// os << "aaaaa"
+ /// << "bbbbb"
+ /// << "\n";
+ /// \endcode
+ BCOS_BetweenStrings,
+ /// Break between adjacent strings that end with \n.
+ /// \code
+ /// os << "aaaaa\n"
+ /// << "bbbbb" << "ccccc\n"
+ /// << "\n";
+ /// \endcode
+ BCOS_BetweenNewlineStrings
+ };
+
+ /// Break Between Chevron Operators
+ /// \version 19
+ BreakChevronOperatorStyle BreakChevronOperator;
+
/// Different ways to break initializers.
enum BreakConstructorInitializersStyle : int8_t {
/// Break constructor initializers before the colon and after the commas.
@@ -4951,6 +4978,7 @@ struct FormatStyle {
BreakBeforeConceptDeclarations == R.BreakBeforeConceptDeclarations &&
BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon &&
BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
+ BreakChevronOperator == R.BreakChevronOperator &&
BreakConstructorInitializers == R.BreakConstructorInitializers &&
BreakFunctionDefinitionParameters ==
R.BreakFunctionDefinitionParameters &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 89e6c19b0af45c..b781a7e161db78 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -243,6 +243,17 @@ struct ScalarEnumerationTraits<FormatStyle::BreakBeforeInlineASMColonStyle> {
}
};
+template <>
+struct ScalarEnumerationTraits<FormatStyle::BreakChevronOperatorStyle> {
+ static void enumeration(IO &IO,
+ FormatStyle::BreakChevronOperatorStyle &Value) {
+ IO.enumCase(Value, "Never", FormatStyle::BCOS_Never);
+ IO.enumCase(Value, "BetweenStrings", FormatStyle::BCOS_BetweenStrings);
+ IO.enumCase(Value, "BetweenNewlineStrings",
+ FormatStyle::BCOS_BetweenNewlineStrings);
+ }
+};
+
template <>
struct ScalarEnumerationTraits<FormatStyle::BreakConstructorInitializersStyle> {
static void
@@ -953,6 +964,7 @@ template <> struct MappingTraits<FormatStyle> {
Style.BreakBeforeInlineASMColon);
IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
+ IO.mapOptional("BreakChevronOperator", Style.BreakChevronOperator);
IO.mapOptional("BreakConstructorInitializers",
Style.BreakConstructorInitializers);
IO.mapOptional("BreakFunctionDefinitionParameters",
@@ -1466,6 +1478,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Always;
LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
LLVMStyle.BreakBeforeTernaryOperators = true;
+ LLVMStyle.BreakChevronOperator = FormatStyle::BCOS_BetweenStrings;
LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
LLVMStyle.BreakFunctionDefinitionParameters = false;
LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 628f70417866c3..ae4bda3511b58d 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5598,10 +5598,18 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
// FIXME: Breaking after newlines seems useful in general. Turn this into an
// option and recognize more cases like endl etc, and break independent of
// what comes after operator lessless.
- if (Right.is(tok::lessless) && Right.Next &&
- Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) &&
- Left.TokenText.ends_with("\\n\"")) {
- return true;
+ if (Style.BreakChevronOperator == FormatStyle::BCOS_BetweenStrings) {
+ if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) &&
+ Right.Next->is(tok::string_literal)) {
+ return true;
+ }
+ }
+ if (Style.BreakChevronOperator == FormatStyle::BCOS_BetweenNewlineStrings) {
+ if (Right.is(tok::lessless) && Right.Next &&
+ Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) &&
+ Left.TokenText.ends_with("\\n\"")) {
+ return true;
+ }
}
if (Right.is(TT_RequiresClause)) {
switch (Style.RequiresClausePosition) {
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 8c74ed2d119a3f..07c070b0338711 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -396,6 +396,14 @@ TEST(ConfigParseTest, ParsesConfiguration) {
CHECK_PARSE("BreakBeforeBinaryOperators: true", BreakBeforeBinaryOperators,
FormatStyle::BOS_All);
+ Style.BreakChevronOperator = FormatStyle::BCOS_BetweenStrings;
+ CHECK_PARSE("BreakChevronOperator: BetweenNewlineStrings",
+ BreakChevronOperator, FormatStyle::BCOS_BetweenNewlineStrings);
+ CHECK_PARSE("BreakChevronOperator: Never", BreakChevronOperator,
+ FormatStyle::BCOS_Never);
+ CHECK_PARSE("BreakChevronOperator: BetweenStrings", BreakChevronOperator,
+ FormatStyle::BCOS_BetweenStrings);
+
Style.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
CHECK_PARSE("BreakConstructorInitializers: BeforeComma",
BreakConstructorInitializers, FormatStyle::BCIS_BeforeComma);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 4906b3350b5b22..566bc9b472d21e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27340,9 +27340,44 @@ TEST_F(FormatTest, PPDirectivesAndCommentsInBracedInit) {
}
TEST_F(FormatTest, StreamOutputOperator) {
- verifyFormat("std::cout << \"foo\" << \"bar\" << baz;");
- verifyFormat("std::cout << \"foo\\n\"\n"
- " << \"bar\";");
+ auto Style = getLLVMStyle();
+
+ // This should be the default as it was the original style, thats
+ // been in place since the beginning.
+ verifyFormat("std::cout << \"aaaa\"\n"
+ " << \"bbbbb\";",
+ Style);
+ verifyFormat("std::cout << \"aaaa\"\n"
+ " << \"bbbbb\"\n"
+ " << \"ccc\";",
+ Style);
+ verifyFormat("std::cout << \"aaaa\"\n"
+ " << \"bbbbb\"\n"
+ " << \"cccc\"\n"
+ " << \"ddd\";",
+ Style);
+ verifyFormat("std::cout << \"aaaa\"\n"
+ " << \"bbbbb\" << baz << \"cccc\"\n"
+ " << \"ddd\";",
+ Style);
+
+ Style.BreakChevronOperator = FormatStyle::BCOS_Never;
+ verifyFormat("std::cout << \"aaaa\" << \"bbb\" << baz;", Style);
+ verifyFormat("std::cout << \"ccc\\n\" << \"dddd\";", Style);
+
+ Style.BreakChevronOperator = FormatStyle::BCOS_BetweenStrings;
+ verifyFormat("std::cout << \"eee\"\n"
+ " << \"ffff\";",
+ Style);
+ verifyFormat("std::cout << \"aa\\n\"\n"
+ " << \"bbbb\";",
+ Style);
+
+ Style.BreakChevronOperator = FormatStyle::BCOS_BetweenNewlineStrings;
+ verifyFormat("std::cout << \"aaaa\" << \"bbb\" << baz;", Style);
+ verifyFormat("std::cout << \"ggg\\n\"\n"
+ " << \"dddd\";",
+ Style);
}
TEST_F(FormatTest, BreakAdjacentStringLiterals) {
@@ -27363,3 +27398,4 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
} // namespace test
} // namespace format
} // namespace clang
+
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Fixes #88483 |
…efault Fixes llvm#88433 A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviours behind a new option.
Fixes llvm#88433 A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviours behind a new option BreakChevronOperator.
Fixes llvm#88433 A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviours behind a new option BreakChevronOperator.
mydeveloperday
changed the title
Fixes llvm#88433 A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviours behind a new option BreakChevronOperator.
clang/lib/Format/TokenAnnotator.cpp
Outdated
| } | ||
| } | ||
| if (Style.BreakChevronOperator == FormatStyle::BCOS_Always) { | ||
| // can be std::os or os |
There was a problem hiding this comment.
I don't understand that comment.
There was a problem hiding this comment.
when you have
os << "A"
<< "B"
you don't want to break between os and << so the loop tracks back to determine if this is the first << in the line, this must also handle std::os << and a::b::os etc.. whats its really saying is don't break before the first <<.
There was a problem hiding this comment.
Yeah ok, but I'd skip the os.
Fixes llvm#88433 A change made to the handling of streaming operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviours behind a new option BreakStreamOperator.
Fixes llvm#88433 A change made to the handling of streaming operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviours behind a new option BreakStreamOperator. - Sort Alphabetically
| } | ||
| case FormatStyle::BCOS_Always: { | ||
| // Don't break after the very first << or >> | ||
| // but the Left token can be os or std::os so |
There was a problem hiding this comment.
Still don't understand the part of os or std::os.
clang/lib/Format/TokenAnnotator.cpp
Outdated
| } | ||
| } | ||
| if (Style.BreakChevronOperator == FormatStyle::BCOS_Always) { | ||
| // can be std::os or os |
There was a problem hiding this comment.
Yeah ok, but I'd skip the os.
|
Please see #88483 (comment) and Conversation in #69859, which was the first attempt at adding an option for breaking consecutive stream insertion operations. Instead of handling everything specified in #69859 (comment) in a single patch, we can take a few steps:
(Ditto for stream extraction operations if needed.) For No. 1 above, an |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes 88433
A change made to the handling of chevron operators causes a large amount of flux in code bases that were previously using clang-format, this fix reverts that change to the default behaviour but adds that new behaviour behind a new option.