[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D69764#2939037 <https://reviews.llvm.org/D69764#2939037>, @HazardyKnusperkeks wrote:

> In D69764#2938973 <https://reviews.llvm.org/D69764#2938973>, @MyDeveloperDay wrote:
>
>> I'm leaning toward introducing <type> into the `CVQualifierOrder` allowing for some qualifiers to be Left of the type and some to be Right (this is not dissimilar to what was discussed some time ago, and similar to the concept of the resharper image above)
>
> Seems reasonable. I would like to remove the `CV`, only `QualifierOrder`. And please take attributes into account, C++ attributes like `[[noreturn]]`, but also compiler attributes like `__attribute__((noreturn))` or `__declspec(noreturn)`, I think it should be possible to add `<attributes>` to your list.

I like both proposals.



================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2101
 
+**CVQualifierAlignment** (``CVQualifierAlignmentStyle``)
+  Different ways to arrange const/volatile qualifiers.
----------------
Why not just `Qualifier` to allow future additions for other qualifiers?
Technically, `static` or `inline` are not qualifiers, but that maybe is clear enough?

Other possibility, `Specifier` (as in "inline specifier")?
OTOH, `Keyword` may be too generic.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2107
+  * ``CVQAS_Leave`` (in configuration: ``Leave``)
+    Don't change cvqualifier to either Left or Right alignment
+
----------------
Nit: `cvqualifier` seems ugly, here and below.


================
Comment at: clang/lib/Format/Format.cpp:2908-2909
+    // Depending on the placement style of left or right you need
+    // To iterate forward or backward though the order list as qualifier
+    // can push though each other.
+    // Copy the very small list and reverse the order if left aligned.
----------------
Nit: typos.


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:201
+      }
+      Next = Next->Next;
+    }
----------------
That will access a nullptr if `Next` in the previous `if` was null. Is that possible BTW? Can we add a test for this? Maybe some possibly invalid code that starts has identifier and coloncolon but doesn't have a template opener? (maybe just `const std::Foo`)?


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

https://reviews.llvm.org/D69764