[PATCH] D32475: [clang-format] Don’t propagate AvoidBinPacking into argument subexpressions

Jacob Bandes-Storch via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 26 11:58:55 PDT 2017


jtbandes added a comment.

Thanks for the feedback, I'll work on making those changes.

In https://reviews.llvm.org/D32475#738425, @djasper wrote:

> What happens if the function call where this happens actually does not have multiple parameters but one parameter with many operands, e.g. changing your test case to:
>
>   arg3____________________________ + is + quite + long + so + it
>       + f(arguments << of << its << subexpressions << lengthy << as << they
>                     << may << or__ << may)


I don't think this is relevant to what I am trying to fix. The issue being fixed is specifically with multiple arguments: I don't want BinPackArguments=false to affect bin-packing of an individual argument. For completeness, though, I can include a test case with one argument.



================
Comment at: lib/Format/ContinuationIndenter.cpp:923
+    // Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
+    if (Current.FakeLParens.size() > 0 &&
+        Current.FakeLParens.back() > prec::Comma) {
----------------
djasper wrote:
> I think you cannot get here if .size() is 0 as this is iterating over the elements.
Good point, thanks.


================
Comment at: unittests/Format/FormatTest.cpp:2596
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
----------------
djasper wrote:
> Is this important?
Not really. I can remove it.


https://reviews.llvm.org/D32475





More information about the cfe-commits mailing list