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

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Thanks for creating the bug report. A few more 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;
----------------
It seems that we set `BK_BracedInit` only on `l_brace`, so no need for a redundant check.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:394-398
+        if (ScopeStart > Start + 1 &&
+            Changes[ScopeStart - 2].Tok->is(tok::identifier) &&
+            Changes[ScopeStart - 1].Tok->is(tok::l_brace) &&
+            Changes[i].Tok->is(tok::l_brace) &&
+            Changes[i].Tok->is(BK_BracedInit))
----------------
Ditto.


================
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"
----------------
Why not just this?


================
Comment at: clang/unittests/Format/FormatTest.cpp:17300-17309
+  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"
----------------
And here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125162