[PATCH] D144709: [clang-format] Improve west to east const
Björn Schäpers via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 24 12:42:48 PST 2023
HazardyKnusperkeks added a comment.
In D144709#4149737 <https://reviews.llvm.org/D144709#4149737>, @AlexanderHederstaf wrote:
> I tried to refactor QualifierAlignmentFixer to handle more types, notably removing the *,&,&& requirement as e.g. const Foo did not work. To avoid moving qualifiers too far, a test for "already east const" is introduced. **However, to get the correct order for the east qualifiers Clang-Format must be used twice.** I assume it's related to how fixes/replacements are handled. Would you like to have a look and perhaps suggest any improvements?
>
> As it must be used twice, some tests related to volatile/const order fail, but I noticed most of them would also fails previously if the type is not simple.
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.
================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:226
+
+ const bool IsEastQualifier = PreviousCheck && [PreviousCheck]() {
+ if (PreviousCheck->is(tok::r_paren)) {
----------------
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.
================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:228
+ if (PreviousCheck->is(tok::r_paren)) {
+ return true;
+ } else if (PreviousCheck->is(TT_TemplateCloser)) {
----------------
Could you add comments what kind of source code we would have here?
================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:229
+ return true;
+ } else if (PreviousCheck->is(TT_TemplateCloser)) {
+ return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
----------------
No `else` after `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,
----------------
That may be null, or not?
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