[PATCH] D144709: [clang-format] Improve left to right const

Björn Schäpers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 26 11:48:15 PST 2023


HazardyKnusperkeks added a comment.

In D144709#4151546 <https://reviews.llvm.org/D144709#4151546>, @AlexanderHederstaf wrote:

>> Highlight by me. That is not acceptable, running the output of `clang-format` through `clang-format` shall not result in any change. In that case it's better to not touch it in any way.
>
> That is no longer an issue in the latest patch.

Great!

Please mark comments as done.

In D144709#4152393 <https://reviews.llvm.org/D144709#4152393>, @MyDeveloperDay wrote:

> when I originally did this work, I checked out the whole fresh area of LLVM and applied the both east then back to west const and built both to ensure I didn't break anything. Please ensure you do the same, I'm going to be honest you are changing it  massively, I'm going to need some time to understand the changes.

>From my point this looks okay, but I did not look very deeply into it, I'll let @MyDeveloperDay do that.



================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:231-232
+    // The case  `Foo() const volatile override` -> `Foo() const volatile
+    // override` The case  `Foo() volatile const final` -> `Foo() const volatile
+    // final`
+    if (PreviousCheck->is(tok::r_paren))
----------------
`clang-format` did wrap your comment ;)


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:230
+    } else if (PreviousCheck->is(TT_TemplateCloser)) {
+      return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
+    } else if (PreviousCheck->isOneOf(TT_PointerOrReference, tok::identifier,
----------------
AlexanderHederstaf wrote:
> HazardyKnusperkeks wrote:
> > That may be null, or not?
> I assumed that if the token is identified as a template closer then there will exist an opener. As far as I am aware, something must preceed that opener.
Then please add an assertion. So we would get a nice message if your assumption is wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709



More information about the cfe-commits mailing list