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

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D69764#2938973 <https://reviews.llvm.org/D69764#2938973>, @MyDeveloperDay wrote:

> With the introduction of `CVQualifierOrder` the need for `CVQualifierAlignment:` doesn't make so much sense
>
>   CVQualifierAlignment: Left
>   CVQualifierOrder: [ "inline", "static", "volatile", "const" ]
>
> 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)
>
> So we would be able to have:
>
>   CVQualifierOrder: [ "inline", "static", "volatile", "<type>", "const", "restrict" ]
>
> By doing so `inline,static,and volatile` are Left and `const,restrict` are Right and so a single setting of Left and Right isn't applicable.
>
> The whole feature could then be determined ONLY using the `CVQualifierOrder` however it doesn't feel like there is a good way to say "Don't change order" (unless of course I use an empty `CVQualifierOrder` list), but this doesn't feel very off by default enough, I like the explicitness of `Leave`.
>
> My gut feeling is to make `Left` and `Right` options define a predetermined CVQualifierOrder, and introduce `Custom` as a new CVQualifierAlignment setting which would require you to define the Order, this has the advantage of driving a sort of semi standard as to what we mean by `Left` and `Right`
>
> Left would by default define:
>
>   CVQualifierOrder: [ "inline", "static", "volatile", "const", "restrict", "<type>"]    
>
> Right would define:
>
>   CVQualifierOrder: [ "inline", "static", "<type>", "volatile", "const", "restrict" ]
>
> (we might want to discuss what these defaults should be, I'm not a language lawyer!)
>
> I realise "inline" and "static" are to the left in the Right alignment, but I'm not sure them being to the right is even legal (its certainly not common) or what is most likely wanted.
>
> I also think if a `CVQualifierOrder` is defined and the `CVQualifierAlignment` is not `Custom` that should be a configuration error to ensure there isn't any ambiguity
>
> Does anyone have any thoughts on this approach?

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.



================
Comment at: clang/lib/Format/Format.cpp:1494
+  // If its empty then it means don't do anything.
+  if (Style->CVQualifierOrder.empty()) {
+    return true;
----------------
Braces :)


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:32
+  if (Err) {
+    llvm::errs() << "Error while rearranging const : "
+                 << llvm::toString(std::move(Err)) << "\n";
----------------
Qualifier?


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:140
+  // We only need to think about streams that begin with const.
+  if (!Tok->is(CVQualifierType)) {
+    return Tok;
----------------
More Braces (follow)


================
Comment at: clang/lib/Format/QualifierAlignmentFixer.h:24
+class QualifierAlignmentFixer : public TokenAnalyzer {
+  std::string CVQualifier;
+
----------------
I would go for only Qualifier.


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

https://reviews.llvm.org/D69764