[PATCH] D125162: [clang-format] fix alignment w/o binpacked args

[cha5on via Phabricator via cfe-commits]

cha5on added inline comments.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:374-375
             return false;
+          if (Changes[i].Tok->is(tok::l_brace) &&
+              Changes[i].Tok->is(BK_BracedInit))
+            return true;
----------------
curdeius wrote:
> It seems that we set `BK_BracedInit` only on `l_brace`, so no need for a redundant check.
`BK_BracedInit` also gets set on `r_brace`, assuming I'm reading `clang/lib/Format/UnwrappedLineParser.cpp` correctly.  I think we need both to be true to be sure that this is the intended token.


================
Comment at: clang/unittests/Format/FormatTest.cpp:17286
+  // BinPackArguments.
+  // See https://github.com/llvm/llvm-project/issues/55360
+  Alignment = getLLVMStyleWithColumns(50);
----------------
HazardyKnusperkeks wrote:
> I'd drop that, or at least replace it with https://llvm.org/PR55360.
Replaced.


================
Comment at: clang/unittests/Format/FormatTest.cpp:17290-17299
+  verifyFormat("int a_long_name = 1;\n"
+               "auto b          = B({a_long_name, a_long_name},\n"
+               "                    {a_longer_name_for_wrap,\n"
+               "                     a_longer_name_for_wrap});",
+               "int a_long_name = 1;\n"
+               "auto b          = B({a_long_name,\n"
+               "                     a_long_name},\n"
----------------
curdeius wrote:
> Why not just this?
I was reading a comment in this file that says that messUp manipulates leading whitespace and thought that meant it wouldn't be appropriate for this.  Upon a closer look, it seems to work fine here, so updating to just provide the expected case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125162/new/

https://reviews.llvm.org/D125162