[clang] [clang-format] revery to string << string handling back to previous default behaviour (PR #88490)

via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 12 07:10:38 PDT 2024


https://github.com/mydeveloperday updated https://github.com/llvm/llvm-project/pull/88490

>From 1c11c3edd0005a729561d84b9a815279b356e8db Mon Sep 17 00:00:00 2001
From: mydeveloperday <mydeveloperday at gmail.com>
Date: Fri, 12 Apr 2024 10:32:19 +0100
Subject: [PATCH 1/4] [clang-format] revery to string << string handling back
 to previous 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.
---
 clang/docs/ClangFormatStyleOptions.rst     | 34 ++++++++++++++++++
 clang/docs/ReleaseNotes.rst                |  2 ++
 clang/include/clang/Format/Format.h        | 28 +++++++++++++++
 clang/lib/Format/Format.cpp                | 13 +++++++
 clang/lib/Format/TokenAnnotator.cpp        | 16 ++++++---
 clang/unittests/Format/ConfigParseTest.cpp |  8 +++++
 clang/unittests/Format/FormatTest.cpp      | 42 ++++++++++++++++++++--
 7 files changed, 136 insertions(+), 7 deletions(-)

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
+                    

>From cbb024ae6ab2e2064ffbee1f46e106a0c3e0b299 Mon Sep 17 00:00:00 2001
From: mydeveloperday <mydeveloperday at gmail.com>
Date: Fri, 12 Apr 2024 11:36:51 +0100
Subject: [PATCH 2/4] [clang-format] revery to string << string handling back
 to previous default behaviour

---
 clang/unittests/Format/FormatTest.cpp | 1 -
 1 file changed, 1 deletion(-)
 mode change 100644 => 100755 clang/unittests/Format/FormatTest.cpp

diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
old mode 100644
new mode 100755
index 566bc9b472d21e..5e5583bf4b234f
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27398,4 +27398,3 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
 } // namespace test
 } // namespace format
 } // namespace clang
-                    

>From 20060add758ebcfd344ffe7aac28b8e2c1ed2c7e Mon Sep 17 00:00:00 2001
From: mydeveloperday <mydeveloperday at gmail.com>
Date: Fri, 12 Apr 2024 15:05:13 +0100
Subject: [PATCH 3/4] [clang-format] revery to string << string handling back
 to previous 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
behaviours behind a new option.
---
 clang/docs/ClangFormatStyleOptions.rst | 10 +++++
 clang/include/clang/Format/Format.h    | 10 ++++-
 clang/lib/Format/Format.cpp            |  2 +
 clang/lib/Format/TokenAnnotator.cpp    | 16 +++++++
 clang/unittests/Format/FormatTest.cpp  | 61 ++++++++++++++++++++++++++
 5 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index a40a940f39d860..9b4187c3a40f64 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3290,6 +3290,16 @@ the configuration (without a prefix: ``Auto``).
          << "bbbbb" << "ccccc\n"
          << "\n";
 
+  * ``BCOS_Always`` (in configuration: ``Always``)
+    Break between adjacent items
+
+    .. code-block:: c++
+
+      os << "aaaaa\n"
+         << "bbbbb"
+         << "ccccc\n"
+         << "\n";
+
 
 
 .. _BreakConstructorInitializers:
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 205c597af8fb0f..0ff9af81bb2f8f 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2213,7 +2213,15 @@ struct FormatStyle {
     ///      << "bbbbb" << "ccccc\n"
     ///      << "\n";
     /// \endcode
-    BCOS_BetweenNewlineStrings
+    BCOS_BetweenNewlineStrings,
+    /// Break between adjacent items
+    /// \code
+    ///   os << "aaaaa\n"
+    ///      << "bbbbb"
+    ///      << "ccccc\n"
+    ///      << "\n";
+    /// \endcode
+    BCOS_Always
   };
 
   /// Break Between Chevron Operators
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index b781a7e161db78..27a12f5ef18f47 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -251,6 +251,7 @@ struct ScalarEnumerationTraits<FormatStyle::BreakChevronOperatorStyle> {
     IO.enumCase(Value, "BetweenStrings", FormatStyle::BCOS_BetweenStrings);
     IO.enumCase(Value, "BetweenNewlineStrings",
                 FormatStyle::BCOS_BetweenNewlineStrings);
+    IO.enumCase(Value, "Always", FormatStyle::BCOS_Always);
   }
 };
 
@@ -4146,3 +4147,4 @@ bool isClangFormatOff(StringRef Comment) {
 
 } // namespace format
 } // namespace clang
+                    
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index ae4bda3511b58d..5ecceb218c5390 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5611,6 +5611,22 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
       return true;
     }
   }
+  if (Style.BreakChevronOperator == FormatStyle::BCOS_Always) {
+    // can be std::os or os
+    auto *FirstChevron = Right.Previous;
+    while (FirstChevron) {
+      if (FirstChevron->isOneOf(tok::lessless, tok::greater))
+        break;
+      FirstChevron = FirstChevron->Previous;
+    }
+
+    if (Right.is(tok::lessless) && FirstChevron)
+      return true;
+    if (Right.is(tok::greater) && Right.Next && Right.Next->is(tok::greater) &&
+        FirstChevron) {
+      return true;
+    }
+  }
   if (Right.is(TT_RequiresClause)) {
     switch (Style.RequiresClausePosition) {
     case FormatStyle::RCPS_OwnLine:
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5e5583bf4b234f..ac620d0a264c48 100755
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27378,6 +27378,67 @@ TEST_F(FormatTest, StreamOutputOperator) {
   verifyFormat("std::cout << \"ggg\\n\"\n"
                "          << \"dddd\";",
                Style);
+
+  Style.BreakChevronOperator = FormatStyle::BCOS_Always;
+  verifyFormat("std::cout << \"aaaa\"\n"
+               "          << \"bbb\"\n"
+               "          << baz;",
+               Style);
+  verifyFormat("std::cout << \"ggg\\n\"\n"
+               "          << \"dddd\";",
+               Style);
+  verifyFormat("std::cout << \"aaaa\"\n"
+               "          << \"bbbbb\"\n"
+               "          << \"cccc\"\n"
+               "          << \"ddd\";",
+               Style);
+  verifyFormat("std::cout << \"aaaa\"\n"
+               "          << \"bbbbb\"\n"
+               "          << baz\n"
+               "          << \"cccc\"\n"
+               "          << \"ddd\";",
+               Style);
+  verifyFormat("cout << \"aaaa\"\n"
+               "     << \"bbbbb\"\n"
+               "     << baz\n"
+               "     << \"cccc\"\n"
+               "     << \"ddd\";",
+               Style);
+}
+
+TEST_F(FormatTest, StreamInputOperator) {
+  auto Style = getLLVMStyle();
+
+  Style.BreakChevronOperator = FormatStyle::BCOS_Never;
+  verifyFormat("std::in >> aaaa >> bbb >> baz;", Style);
+  verifyFormat("std::in >> ccc >> dddd;", Style);
+
+  Style.BreakChevronOperator = FormatStyle::BCOS_Always;
+  verifyFormat("std::in >> ccc;", Style);
+  verifyFormat("std::in >> aaaa\n"
+               "    >> bbb\n"
+               "    >> baz;",
+               Style);
+  verifyFormat("std::in >> ggg\n"
+               "    >> dddd;",
+               Style);
+  verifyFormat("std::in >> aaaa\n"
+               "    >> bbbbb\n"
+               "    >> cccc\n"
+               "    >> ddd;",
+               Style);
+  verifyFormat("std::in >> aaaa\n"
+               "    >> bbbbb\n"
+               "    >> baz\n"
+               "    >> cccc\n"
+               "    >> ddd;",
+               Style);
+  verifyFormat("in >> aaaa\n"
+               "    >> bbbbb\n"
+               "    >> baz\n"
+               "    >> cccc\n"
+               "    >> ddd;",
+               Style);
 }
 
 TEST_F(FormatTest, BreakAdjacentStringLiterals) {

>From ada5b57dda2a818b9a6404e67fcbcdf66a4b5b28 Mon Sep 17 00:00:00 2001
From: mydeveloperday <mydeveloperday at gmail.com>
Date: Fri, 12 Apr 2024 15:09:29 +0100
Subject: [PATCH 4/4] [clang-format] revery to string << string handling to
 previous 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
behaviours behind a new option BreakChevronOperator.
---
 clang/docs/ClangFormatStyleOptions.rst | 2 +-
 clang/include/clang/Format/Format.h    | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 9b4187c3a40f64..8c8326285597db 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3291,7 +3291,7 @@ the configuration (without a prefix: ``Auto``).
          << "\n";
 
   * ``BCOS_Always`` (in configuration: ``Always``)
-    Break between adjacent items
+    Break between adjacent chevrons.
 
     .. code-block:: c++
 
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 0ff9af81bb2f8f..b0bd6fea9b7904 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2193,7 +2193,7 @@ struct FormatStyle {
   /// \version 3.7
   bool BreakBeforeTernaryOperators;
 
-  /// Different ways to Break Between Chevrons
+  /// Different ways to Break Between Chevrons.
   enum BreakChevronOperatorStyle : int8_t {
     /// Break using ColumnLimit rules.
     /// \code
@@ -2214,7 +2214,7 @@ struct FormatStyle {
     ///      << "\n";
     /// \endcode
     BCOS_BetweenNewlineStrings,
-    /// Break between adjacent items
+    /// Break between adjacent chevrons.
     /// \code
     ///   os << "aaaaa\n"
     ///      << "bbbbb"



More information about the cfe-commits mailing list