MyDeveloperDay added subscribers: sammccall, djasper, klimek.
MyDeveloperDay added inline comments.
================
Comment at: clang/unittests/Format/FormatTest.cpp:6636
- verifyFormat("aaaaaaaaa *a = aaaaaaaaaaaaaaaaaaa, *b =
bbbbbbbbbbbbbbbbbbb,\n"
- " *b = bbbbbbbbbbbbbbbbbbb, *d = ddddddddddddddddddd;",
Style);
----------------
arichardson wrote:
> 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.
Again how is this even left alignment anyway!
================
Comment at: clang/unittests/Format/FormatTest.cpp:6636
+ verifyFormat("aaaaaaaaa* a = aaaaaaaaaaaaaaaaaaa, * b =
bbbbbbbbbbbbbbbbbb,\n"
+ " * b = bbbbbbbbbbbbbbbbbbb, * d = dddddddddddddddd;",
Style);
----------------
MyDeveloperDay wrote:
> arichardson wrote:
> > 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.
> Again how is this even left alignment anyway!
I'm looking back an @djasper change, and it seems we are basically reverting
the behavior it so I feel a little uncomfortable, I'm not sure how to proceed,
having said that I think that change was also wrong because its supposed to be
left aligned and basically its not!
At this point I'd personally prefer to wait for a higher power, maybe if
@sammccall or @klimek are listening they could comment.
I'm going to pull the review into my local branch and see what impact it has on
the code I look after. I also often run my code through those areas of clang
that are fully clang-formatted to see what the impact will be.
================
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);
+ verifyFormat("char** x, ** y = NULL;", Style);
+ verifyFormat("int x, * xp = &x, & xr = x, *& xpr = xp, * const& xcpr = xp;",
+ Style);
+ Style.PointerAlignment = FormatStyle::PAS_Right;
verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
}
----------------
arichardson wrote:
> 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).
Actually this case is odd, because its supposed to be left aligned and we are
getting right alignment anyway... ignore my comment on the right
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88239/new/
https://reviews.llvm.org/D88239
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
