[clang] [clang-format] Remove special handling of comments after brace/paren (PR #71672)

Björn Schäpers via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 8 04:36:04 PST 2023


https://github.com/HazardyKnusperkeks created https://github.com/llvm/llvm-project/pull/71672

Fixes http://llvm.org/PR55487

The code did not match the documentation about Cpp11BracedListStyle. Changed handling of comments after opening braces, which are supposedly function call like to behave exactly like their parenthesis counter part.

>From 4548aa1186531fb93a623144278fe72896d47e89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Sch=C3=A4pers?= <bjoern at hazardy.de>
Date: Wed, 8 Nov 2023 13:31:32 +0100
Subject: [PATCH] [clang-format] Remove special handling of comments after
 brace/paren

Fixes http://llvm.org/PR55487

The code did not match the documentation about Cpp11BracedListStyle.
Changed handling of comments after opening braces, which are supposedly
function call like to behave exactly like their parenthesis counter
part.
---
 clang/lib/Format/ContinuationIndenter.cpp     |  4 ++-
 clang/lib/Format/TokenAnnotator.cpp           |  8 ++---
 clang/unittests/Format/FormatTest.cpp         | 24 +++++++--------
 clang/unittests/Format/FormatTestComments.cpp | 29 +++++++++++++++----
 4 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index 3a829cdedb77fc7..1d76dbe39b00eae 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -802,7 +802,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
       Previous.isNot(TT_ObjCMethodExpr) && Previous.isNot(TT_RequiresClause) &&
       !(Current.MacroParent && Previous.MacroParent) &&
       (Current.isNot(TT_LineComment) ||
-       Previous.isOneOf(BK_BracedInit, TT_VerilogMultiLineListLParen))) {
+       (Previous.is(BK_BracedInit) &&
+        (!Previous.Previous || Previous.Previous->isNot(tok::identifier))) ||
+       Previous.is(TT_VerilogMultiLineListLParen))) {
     CurrentState.Indent = State.Column + Spaces;
     CurrentState.IsAligned = true;
   }
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 729e7e370bf62ea..9691d879735d3aa 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3510,12 +3510,10 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
   while (Current) {
     const FormatToken *Prev = Current->Previous;
     if (Current->is(TT_LineComment)) {
-      if (Prev->is(BK_BracedInit) && Prev->opensScope()) {
+      /*if (Prev->is(BK_BracedInit) && Prev->opensScope()) {
         Current->SpacesRequiredBefore =
-            (Style.Cpp11BracedListStyle && !Style.SpacesInParensOptions.Other)
-                ? 0
-                : 1;
-      } else if (Prev->is(TT_VerilogMultiLineListLParen)) {
+            Style.SpacesInParensOptions.Other ? 1 : 0;
+      } else */if (Prev->is(TT_VerilogMultiLineListLParen)) {
         Current->SpacesRequiredBefore = 0;
       } else {
         Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 80903e7630c8073..e47c27c6b728c80 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -13500,20 +13500,18 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
                "                                CDDDP83848_RBR_REGISTER};",
                NoBinPacking);
 
-  // FIXME: The alignment of these trailing comments might be bad. Then again,
-  // this might be utterly useless in real code.
   verifyFormat("Constructor::Constructor()\n"
-               "    : some_value{         //\n"
-               "                 aaaaaaa, //\n"
-               "                 bbbbbbb} {}");
+               "    : some_value{  //\n"
+               "          aaaaaaa, //\n"
+               "          bbbbbbb} {}");
 
   // In braced lists, the first comment is always assumed to belong to the
   // first element. Thus, it can be moved to the next or previous line as
   // appropriate.
-  verifyFormat("function({// First element:\n"
-               "          1,\n"
-               "          // Second element:\n"
-               "          2});",
+  verifyFormat("function({ // First element:\n"
+               "           1,\n"
+               "           // Second element:\n"
+               "           2});",
                "function({\n"
                "    // First element:\n"
                "    1,\n"
@@ -13582,9 +13580,9 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat(
       "someFunction(OtherParam,\n"
       "             BracedList{ // comment 1 (Forcing interesting break)\n"
-      "                         param1, param2,\n"
-      "                         // comment 2\n"
-      "                         param3, param4 });",
+      "                 param1, param2,\n"
+      "                 // comment 2\n"
+      "                 param3, param4 });",
       ExtraSpaces);
   verifyFormat(
       "std::this_thread::sleep_for(\n"
@@ -13635,7 +13633,7 @@ TEST_F(FormatTest, LayoutCxx11BraceInitializers) {
   verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
   verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
   verifyFormat("vector< int > x{ // comment 1\n"
-               "                 1, 2, 3, 4 };",
+               "    1, 2, 3, 4 };",
                SpaceBetweenBraces);
   SpaceBetweenBraces.ColumnLimit = 20;
   verifyFormat("vector< int > x{\n"
diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp
index 967ffa32db79c75..d971744a108b531 100644
--- a/clang/unittests/Format/FormatTestComments.cpp
+++ b/clang/unittests/Format/FormatTestComments.cpp
@@ -1376,12 +1376,12 @@ TEST_F(FormatTestComments, CommentsInStaticInitializers) {
   verifyFormat("S s = {{a, b, c},  // Group #1\n"
                "       {d, e, f},  // Group #2\n"
                "       {g, h, i}}; // Group #3");
-  verifyFormat("S s = {{// Group #1\n"
-               "        a, b, c},\n"
-               "       {// Group #2\n"
-               "        d, e, f},\n"
-               "       {// Group #3\n"
-               "        g, h, i}};");
+  verifyFormat("S s = {{ // Group #1\n"
+               "         a, b, c},\n"
+               "       { // Group #2\n"
+               "         d, e, f},\n"
+               "       { // Group #3\n"
+               "         g, h, i}};");
 
   EXPECT_EQ("S s = {\n"
             "    // Some comment\n"
@@ -4576,6 +4576,23 @@ TEST_F(FormatTestComments, SplitCommentIntroducers) {
                    getLLVMStyleWithColumns(10)));
 }
 
+TEST_F(FormatTestComments, LineCommentsOnStartOfFunctionCall) {
+  verifyFormat("T foo( // Comment\n"
+               "    arg);");
+
+  verifyFormat("T bar{ // Comment\n"
+               "    arg};");
+
+  verifyFormat("T baz({ // Comment\n"
+               "        arg});");
+
+  verifyFormat("func( // Comment\n"
+               "    arg);");
+
+  verifyFormat("func({ // Comment\n"
+               "       arg});");
+}
+
 } // end namespace
 } // namespace test
 } // end namespace format



More information about the cfe-commits mailing list