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