[PATCH] D88239: [clang-format] Fix spaces around */& in multi-variable declarations

Alexander Richardson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 24 15:31:59 PDT 2020


arichardson added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:6636
+  verifyFormat("aaaaaaaaa* a = aaaaaaaaaaaaaaaaaaa, * b = bbbbbbbbbbbbbbbbbb,\n"
+               "           * b = bbbbbbbbbbbbbbbbbbb, * d = dddddddddddddddd;",
                Style);
----------------
MyDeveloperDay wrote:
> huh! the original tests doesn't look like LEFT aligned but now we also lose the alignment, should we care I wonder?
I looked at the git log and it seems like it was changed from left to right in https://github.com/llvm/llvm-project/commit/bea1ab46d9ffdfc50108580c712596a54323a94c


================
Comment at: clang/unittests/Format/FormatTest.cpp:6636
+  verifyFormat("aaaaaaaaa* a = aaaaaaaaaaaaaaaaaaa, * b = bbbbbbbbbbbbbbbbbb,\n"
+               "           * b = bbbbbbbbbbbbbbbbbbb, * d = dddddddddddddddd;",
                Style);
----------------
arichardson wrote:
> MyDeveloperDay wrote:
> > huh! the original tests doesn't look like LEFT aligned but now we also lose the alignment, should we care I wonder?
> I looked at the git log and it seems like it was changed from left to right in https://github.com/llvm/llvm-project/commit/bea1ab46d9ffdfc50108580c712596a54323a94c
Losing the alignment is annoying but I don't quite understand why it changed. Hopefully there aren't too many multi line multi variable declarations.


================
Comment at: clang/unittests/Format/FormatTest.cpp:6639
   verifyFormat("vector<int*> a, b;", Style);
+  verifyFormat("for (int* p, * q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char* x, * const* y = NULL;", Style);
----------------
MyDeveloperDay wrote:
> I don't get why `*q` would be `* q`
Yeah I'm not sure what the best way to format multi-variable declarations with left alignment. We usually have a space before the name so I thought I'd do the same here (even though it looks a bit odd).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88239



More information about the cfe-commits mailing list