r257325 - clang-format: Slightly row back on r257257.

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 11 03:01:05 PST 2016


Author: djasper
Date: Mon Jan 11 05:01:05 2016
New Revision: 257325

URL: http://llvm.org/viewvc/llvm-project?rev=257325&view=rev
Log:
clang-format: Slightly row back on r257257.

r257257 change the way clang-format enforces line breaks after a
templated type has been line-wrapped. This was to fix an incorrect line
break if BinPackParameters is set to false. However, it also leads to
an unwanted line break in a different case. Thus, for now, only do this
when BinPackParameters is false. This isn't ideal yet, but helps us
until we have a better solution.

With BinPackParameters:
Before:
  void fffffffffff(aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa,
                                               aaaaaaaaaa> aaaaaaaaaa);

After:
  void fffffffffff(
      aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaa>
          aaaaaaaaaa);

Modified:
    cfe/trunk/lib/Format/ContinuationIndenter.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=257325&r1=257324&r2=257325&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon Jan 11 05:01:05 2016
@@ -151,7 +151,11 @@ bool ContinuationIndenter::mustBreak(con
     return true;
   if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
        (Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
-        Previous.NestingLevel == 1) ||
+        // FIXME: This is a temporary workaround for the case where clang-format
+        // sets BreakBeforeParameter to avoid bin packing and this creates a
+        // completely unnecessary line break after a template type that isn't
+        // line-wrapped.
+        (Previous.NestingLevel == 1 || Style.BinPackParameters)) ||
        (Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&
         Previous.isNot(tok::question)) ||
        (!Style.BreakBeforeTernaryOperators &&

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=257325&r1=257324&r2=257325&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 11 05:01:05 2016
@@ -4062,10 +4062,23 @@ TEST_F(FormatTest, FormatsDeclarationsOn
                "       int aaaaaaaaaaaaaaaaaaaa,\n"
                "       int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}",
                NoBinPacking);
+
   NoBinPacking.AllowAllParametersOfDeclarationOnNextLine = false;
   verifyFormat("void aaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
                "                        vector<int> bbbbbbbbbbbbbbb);",
                NoBinPacking);
+  // FIXME: This behavior difference is probably not wanted. However, currently
+  // we cannot distinguish BreakBeforeParameter being set because of the wrapped
+  // template arguments from BreakBeforeParameter being set because of the
+  // one-per-line formatting.
+  verifyFormat(
+      "void fffffffffff(aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa,\n"
+      "                                             aaaaaaaaaa> aaaaaaaaaa);",
+      NoBinPacking);
+  verifyFormat(
+      "void fffffffffff(\n"
+      "    aaaaaaaaaaaaaaaaaaaaaaaaaaa<aaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaa>\n"
+      "        aaaaaaaaaa);");
 }
 
 TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {




More information about the cfe-commits mailing list