[PATCH] D103245: [clang-format] Fix PointerAlignmentRight with AlignConsecutiveDeclarations

[Gerhard Gappmeier via Phabricator via cfe-commits]

gergap marked 4 inline comments as done.
gergap added inline comments.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:369
     assert(Shift >= 0);
+    if (Shift == 0)
+      continue;
----------------
HazardyKnusperkeks wrote:
> 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.
I actually don't know. Just copied this from the original patch.


================
Comment at: clang/unittests/Format/FormatTest.cpp:14884
   Alignment.AlignConsecutiveDeclarations = FormatStyle::ACS_None;
+  Alignment.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("float const a = 5;\n"
----------------
HazardyKnusperkeks wrote:
> Why change this?
It is already set to PAS_Right by default. But when reading the code you don't know this. I needed to debug this to find this out.
The following verifyFormat tests depend on PAS_Right style.


================
Comment at: clang/unittests/Format/FormatTest.cpp:15045
+  // PAS_RIGHT
   EXPECT_EQ("void SomeFunction(int parameter = 0) {\n"
             "  int const i   = 1;\n"
----------------
HazardyKnusperkeks wrote:
> 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.
I don't know, because I'm not the author of that code.
But I can change it to verifyFormat if this is what you prefer.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103245