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