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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
