[PATCH] D127873: [clang-format] Fix misplacemnt of `*` in declartion of pointer to struct

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 16 03:31:38 PDT 2022


MyDeveloperDay added inline comments.


================
Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:93
+  EXPECT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::star, TT_PointerOrReference);
 }
----------------
HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > Can you add a verifyFormat test that shows what you want? as well
> I think the annotator test is sufficient. Because it's just about annotating the token, formatting is secondary (and dependent on style - these tests are already in place).
I know where you are coming from, but actually if it wasn't for the `enable_if<>{} && ...` example in the unit tests then we'd have missed the && case and caused a regression. (that was ONLY covered by the small example code snippet)

Whilst I believe the TokenAnnotator tests are correct to have as well, I think we should be adding formatting examples just to ensure someone doesn't breaking the formatting rule that this depends on.  I always follow the Beyoncé rule... "If you like it you should have put a test on it!"

So If you don't want anyone to break your `*` placement after the `}` then I see no harm in adding a single verifyFormat(), but while you are at it please also test to ensure the Left/Middle/Right works with your example as you might expect. (just incase there are some rules about that, that could interfere with your setting)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127873



More information about the cfe-commits mailing list