[clang] [clang-format] Add BreakBeforeTemplateClose option (PR #118046)

via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 30 13:25:02 PST 2024


https://github.com/leijurv updated https://github.com/llvm/llvm-project/pull/118046

>From 1caf823165b16f6701993d586df51d5cdbf0885e Mon Sep 17 00:00:00 2001
From: Leijurv <leijurv at gmail.com>
Date: Fri, 29 Nov 2024 21:54:36 -0600
Subject: [PATCH] [clang-format] Add BreakBeforeTemplateClose option

---
 clang/docs/ClangFormatStyleOptions.rst     |  21 ++++
 clang/docs/ReleaseNotes.rst                |   1 +
 clang/include/clang/Format/Format.h        |  20 ++++
 clang/lib/Format/ContinuationIndenter.cpp  |  11 ++
 clang/lib/Format/ContinuationIndenter.h    |  26 ++--
 clang/lib/Format/Format.cpp                |   2 +
 clang/lib/Format/TokenAnnotator.cpp        |   2 +-
 clang/unittests/Format/ConfigParseTest.cpp |   1 +
 clang/unittests/Format/FormatTest.cpp      | 131 +++++++++++++++++++++
 9 files changed, 206 insertions(+), 9 deletions(-)

diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 4be448171699ca..84ab1b0a2eff61 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3416,6 +3416,27 @@ the configuration (without a prefix: ``Auto``).
 
 
 
+.. _BreakBeforeTemplateClose:
+
+**BreakBeforeTemplateClose** (``Boolean``) :versionbadge:`clang-format 20` :ref:`¶ <BreakBeforeTemplateClose>`
+  If ``true``, a line break will be placed before the ``>`` in a multiline
+  template declaration.
+
+  .. code-block:: c++
+
+     true:
+     template <
+         typename Foo,
+         typename Bar,
+         typename Baz
+     >
+
+     false:
+     template <
+         typename Foo,
+         typename Bar,
+         typename Baz>
+
 .. _BreakBeforeTernaryOperators:
 
 **BreakBeforeTernaryOperators** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <BreakBeforeTernaryOperators>`
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e44aefa90ab386..867d4b5d8c3f18 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -976,6 +976,7 @@ clang-format
   ``Never``, and ``true`` to ``Always``.
 - Adds ``RemoveEmptyLinesInUnwrappedLines`` option.
 - Adds ``KeepFormFeed`` option and set it to ``true`` for ``GNU`` style.
+- Adds ``BreakBeforeTemplateClose`` option.
 
 libclang
 --------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 6383934afa2c40..bffd964f6aa8aa 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2248,6 +2248,25 @@ struct FormatStyle {
   /// \version 16
   BreakBeforeInlineASMColonStyle BreakBeforeInlineASMColon;
 
+  /// If ``true``, a line break will be placed before the ``>`` in a multiline
+  /// template declaration.
+  /// \code
+  ///    true:
+  ///    template <
+  ///        typename Foo,
+  ///        typename Bar,
+  ///        typename Baz
+  ///    >
+  ///
+  ///    false:
+  ///    template <
+  ///        typename Foo,
+  ///        typename Bar,
+  ///        typename Baz>
+  /// \endcode
+  /// \version 20
+  bool BreakBeforeTemplateClose;
+
   /// If ``true``, ternary operators will be placed after line breaks.
   /// \code
   ///    true:
@@ -5184,6 +5203,7 @@ struct FormatStyle {
            BreakBeforeBraces == R.BreakBeforeBraces &&
            BreakBeforeConceptDeclarations == R.BreakBeforeConceptDeclarations &&
            BreakBeforeInlineASMColon == R.BreakBeforeInlineASMColon &&
+           BreakBeforeTemplateClose == R.BreakBeforeTemplateClose &&
            BreakBeforeTernaryOperators == R.BreakBeforeTernaryOperators &&
            BreakBinaryOperations == R.BreakBinaryOperations &&
            BreakConstructorInitializers == R.BreakConstructorInitializers &&
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index aed86c1fb99551..4c783623afc535 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -406,6 +406,10 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
   }
   if (CurrentState.BreakBeforeClosingParen && Current.is(tok::r_paren))
     return true;
+  if (CurrentState.BreakBeforeClosingAngle &&
+      Current.ClosesTemplateDeclaration && Style.BreakBeforeTemplateClose) {
+    return true;
+  }
   if (Style.Language == FormatStyle::LK_ObjC &&
       Style.ObjCBreakBeforeNestedBlockParam &&
       Current.ObjCSelectorNameParts > 1 &&
@@ -1234,6 +1238,9 @@ unsigned ContinuationIndenter::addTokenOnNewLine(LineState &State,
         Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent;
   }
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::less))
+    CurrentState.BreakBeforeClosingAngle = true;
+
   if (CurrentState.AvoidBinPacking) {
     // If we are breaking after '(', '{', '<', or this is the break after a ':'
     // to start a member initializer list in a constructor, this should not
@@ -1370,6 +1377,10 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
       State.Stack.size() > 1) {
     return State.Stack[State.Stack.size() - 2].LastSpace;
   }
+  if (Current.ClosesTemplateDeclaration && Style.BreakBeforeTemplateClose &&
+      State.Stack.size() > 1) {
+    return State.Stack[State.Stack.size() - 2].LastSpace;
+  }
   if (NextNonComment->is(TT_TemplateString) && NextNonComment->closesScope())
     return State.Stack[State.Stack.size() - 2].LastSpace;
   // Field labels in a nested type should be aligned to the brace. For example
diff --git a/clang/lib/Format/ContinuationIndenter.h b/clang/lib/Format/ContinuationIndenter.h
index 18441e10a12492..88d214473396a8 100644
--- a/clang/lib/Format/ContinuationIndenter.h
+++ b/clang/lib/Format/ContinuationIndenter.h
@@ -200,14 +200,15 @@ struct ParenState {
       : Tok(Tok), Indent(Indent), LastSpace(LastSpace),
         NestedBlockIndent(Indent), IsAligned(false),
         BreakBeforeClosingBrace(false), BreakBeforeClosingParen(false),
-        AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),
-        NoLineBreak(NoLineBreak), NoLineBreakInOperand(false),
-        LastOperatorWrapped(true), ContainsLineBreak(false),
-        ContainsUnwrappedBuilder(false), AlignColons(true),
-        ObjCSelectorNameFound(false), HasMultipleNestedBlocks(false),
-        NestedBlockInlined(false), IsInsideObjCArrayLiteral(false),
-        IsCSharpGenericTypeConstraint(false), IsChainedConditional(false),
-        IsWrappedConditional(false), UnindentOperator(false) {}
+        BreakBeforeClosingAngle(false), AvoidBinPacking(AvoidBinPacking),
+        BreakBeforeParameter(false), NoLineBreak(NoLineBreak),
+        NoLineBreakInOperand(false), LastOperatorWrapped(true),
+        ContainsLineBreak(false), ContainsUnwrappedBuilder(false),
+        AlignColons(true), ObjCSelectorNameFound(false),
+        HasMultipleNestedBlocks(false), NestedBlockInlined(false),
+        IsInsideObjCArrayLiteral(false), IsCSharpGenericTypeConstraint(false),
+        IsChainedConditional(false), IsWrappedConditional(false),
+        UnindentOperator(false) {}
 
   /// \brief The token opening this parenthesis level, or nullptr if this level
   /// is opened by fake parenthesis.
@@ -280,6 +281,13 @@ struct ParenState {
   /// was a newline after the beginning left paren.
   bool BreakBeforeClosingParen : 1;
 
+  /// Whether a newline needs to be inserted before the block's closing
+  /// angle < >.
+  ///
+  /// We only want to insert a newline before the closing angle if there also
+  /// was a newline after the beginning left angle.
+  bool BreakBeforeClosingAngle : 1;
+
   /// Avoid bin packing, i.e. multiple parameters/elements on multiple
   /// lines, in this context.
   bool AvoidBinPacking : 1;
@@ -367,6 +375,8 @@ struct ParenState {
       return BreakBeforeClosingBrace;
     if (BreakBeforeClosingParen != Other.BreakBeforeClosingParen)
       return BreakBeforeClosingParen;
+    if (BreakBeforeClosingAngle != Other.BreakBeforeClosingAngle)
+      return BreakBeforeClosingAngle;
     if (QuestionColumn != Other.QuestionColumn)
       return QuestionColumn < Other.QuestionColumn;
     if (AvoidBinPacking != Other.AvoidBinPacking)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index ee52972ce66f4a..d6aa80dd8e3aef 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1000,6 +1000,7 @@ template <> struct MappingTraits<FormatStyle> {
     IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
     IO.mapOptional("BreakBeforeInlineASMColon",
                    Style.BreakBeforeInlineASMColon);
+    IO.mapOptional("BreakBeforeTemplateClose", Style.BreakBeforeTemplateClose);
     IO.mapOptional("BreakBeforeTernaryOperators",
                    Style.BreakBeforeTernaryOperators);
     IO.mapOptional("BreakBinaryOperations", Style.BreakBinaryOperations);
@@ -1514,6 +1515,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Always;
   LLVMStyle.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline;
+  LLVMStyle.BreakBeforeTemplateClose = false;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBinaryOperations = FormatStyle::BBO_Never;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index bc5239209f3aab..c9908515833bad 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -6254,7 +6254,7 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
     return false;
 
   if (Right.is(TT_TemplateCloser))
-    return false;
+    return Right.ClosesTemplateDeclaration && Style.BreakBeforeTemplateClose;
   if (Right.is(tok::r_square) && Right.MatchingParen &&
       Right.MatchingParen->is(TT_LambdaLSquare)) {
     return false;
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 7fc7492271668b..39f4ea49719600 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -162,6 +162,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BreakAdjacentStringLiterals);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
+  CHECK_PARSE_BOOL(BreakBeforeTemplateClose);
   CHECK_PARSE_BOOL(BreakBeforeTernaryOperators);
   CHECK_PARSE_BOOL(BreakStringLiterals);
   CHECK_PARSE_BOOL(CompactNamespaces);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 250e51b5421664..07205b010575de 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -11077,6 +11077,137 @@ TEST_F(FormatTest, WrapsTemplateDeclarationsWithComments) {
       Style);
 }
 
+TEST_F(FormatTest, BreakBeforeTemplateClose) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Cpp);
+  Style.ColumnLimit = 0;
+  verifyNoChange("template <typename Foo>\n"
+                 "void foo() {}",
+                 Style);
+  verifyNoChange("template <\n"
+                 "    typename Foo,\n"
+                 "    typename Bar>\n"
+                 "void foo() {}",
+                 Style);
+  // when BreakBeforeTemplateClose is off, this line break is removed:
+  verifyFormat("template <\n"
+               "    typename Foo,\n"
+               "    typename Bar>\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               Style);
+  Style.BreakBeforeTemplateClose = true;
+  // BreakBeforeTemplateClose should NOT force multiline templates
+  verifyNoChange("template <typename Foo>\n"
+                 "void foo() {}",
+                 Style);
+  verifyNoChange("template <typename Foo, typename Bar>\n"
+                 "void foo() {}",
+                 Style);
+  // it should allow a line break:
+  verifyNoChange("template <\n"
+                 "    typename Foo\n"
+                 ">\n"
+                 "void foo() {}",
+                 Style);
+  verifyNoChange("template <\n"
+                 "    typename Foo,\n"
+                 "    typename Bar\n"
+                 ">\n"
+                 "void foo() {}",
+                 Style);
+  // it should add a line break if not already present:
+  verifyFormat("template <\n"
+               "    typename Foo\n"
+               ">\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo>\n"
+               "void foo() {}",
+               Style);
+  verifyFormat("template <\n"
+               "    typename Foo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo,\n"
+               "    typename Bar>\n"
+               "void foo() {}",
+               Style);
+  // when within an indent scope, the > should be placed appropriately:
+  verifyFormat("struct Baz {\n"
+               "  template <\n"
+               "      typename Foo,\n"
+               "      typename Bar\n"
+               "  >\n"
+               "  void foo() {}\n"
+               "};",
+               "struct Baz {\n"
+               "  template <\n"
+               "      typename Foo,\n"
+               "      typename Bar>\n"
+               "  void foo() {}\n"
+               "};",
+               Style);
+
+  // now test that it handles the cases when the column limit forces wrapping
+  Style.ColumnLimit = 40;
+  // when the column limit allows it, the template should be combined back into
+  // one line:
+  verifyFormat("template <typename Foo, typename Bar>\n"
+               "void foo() {}",
+               "template <\n"
+               "    typename Foo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               Style);
+  // but not when the name is looong
+  verifyFormat("template <\n"
+               "    typename Foo,\n"
+               "    typename Barrrrrrrrrrrrrrrrrrrrrrrrrr\n"
+               ">\n"
+               "void foo() {}",
+               Style);
+  verifyFormat("template <\n"
+               "    typename Fooooooooooooooooooooooooooo,\n"
+               "    typename Bar\n"
+               ">\n"
+               "void foo() {}",
+               Style);
+  // additionally, long names should be split in one step:
+  verifyFormat(
+      "template <\n"
+      "    typename Foo,\n"
+      "    typename Barrrrrrrrrrrrrrrrrrrrrrrrrr\n"
+      ">\n"
+      "void foo() {}",
+      "template <typename Foo, typename Barrrrrrrrrrrrrrrrrrrrrrrrrr>\n"
+      "void foo() {}",
+      Style);
+  verifyFormat(
+      "template <\n"
+      "    typename Fooooooooooooooooooooooooooo,\n"
+      "    typename Bar\n"
+      ">\n"
+      "void foo() {}",
+      "template <typename Fooooooooooooooooooooooooooo, typename Bar>\n"
+      "void foo() {}",
+      Style);
+  // even when there is only one long name:
+  verifyFormat("template <\n"
+               "    typename Fooooooooooooooooooooooooooo\n"
+               ">\n"
+               "void foo() {}",
+               "template <typename Fooooooooooooooooooooooooooo>\n"
+               "void foo() {}",
+               Style);
+}
+
 TEST_F(FormatTest, WrapsTemplateParameters) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;



More information about the cfe-commits mailing list