HazardyKnusperkeks added a comment.
Looks quite solid for me.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:265
static void
-AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
+AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
+ unsigned Column, F &&Matches,
----------------
I don't know if we should insert the `Style`, maybe just the `PointerAlignment`?
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:369
assert(Shift >= 0);
+ if (Shift == 0)
+ continue;
----------------
This is unrelated, isn't it?
If it is, I would like a separate commit. Otherwise an explanation why it is
now mandatory and does not infer with the other alignments.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:382
+ Changes[previous].Tok->getType() == TT_PointerOrReference;
+ previous--) {
+ Changes[previous + 1].Spaces -= Shift;
----------------
I like prefix better than postfix. :)
================
Comment at: clang/unittests/Format/FormatTest.cpp:14884
Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+ Alignment.PointerAlignment = FormatStyle::PAS_Right;
verifyFormat("float const a = 5;\n"
----------------
Why change this?
================
Comment at: clang/unittests/Format/FormatTest.cpp:15045
+ // PAS_RIGHT
EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
" int const i = 1;\n"
----------------
I don't know why this is `EXPECT_EQ` instead of `verifyFormat`, but I know
someone who will request that. :)
You should change that here and use that for your following tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103245/new/
https://reviews.llvm.org/D103245
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
