[PATCH] D144709: [clang-format] Improve west to east const

[Alexander Hederstaf via Phabricator via cfe-commits]

AlexanderHederstaf added a comment.

Thanks for the review so far!

Passes all tests and works in one shot on my test files as well as 3k+ files repository, i.e. it does not have to be run twice.

As one use-case of this rule is to be able to checkout code and freely swap between left/right. Then the left alignment would also need to handle the same cases as right alignment.

I should also make templates with requires work, though I am not familiar with the variations that syntax may have.

@rymiel Good, this might take a few more iterations.



================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:226
+
+  const bool IsEastQualifier = PreviousCheck && [PreviousCheck]() {
+    if (PreviousCheck->is(tok::r_paren)) {
----------------
HazardyKnusperkeks wrote:
> In the initial commit concerns were raised that `East` and `West` are westcentric terms and should not be used. Thus you should stick here to `Left` and `Right` the same terms we use in the configuration.
Replaced with right/left


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:228
+    if (PreviousCheck->is(tok::r_paren)) {
+      return true;
+    } else if (PreviousCheck->is(TT_TemplateCloser)) {
----------------
HazardyKnusperkeks wrote:
> Could you add comments what kind of source code we would have here?
Added comments for the different source code we may have.


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:229
+      return true;
+    } else if (PreviousCheck->is(TT_TemplateCloser)) {
+      return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
----------------
HazardyKnusperkeks wrote:
> No `else` after `return`.
removed 'else' preceeding return.


================
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,
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709