HazardyKnusperkeks added a comment. I already said I would like that in `clang-format` and would directly add that to my config.
I also think that there should be no problem in having that in `clang-format`, include sorting has a bigger chance of breaking code, yeah only with poorly designed headers but these exist, something which can not be fixed in clang-format itself, but only with `// clang-format off` or a elaborated sorting configuration. Breaking `const` moves can be fixed in `clang-format` and since it is off by default I really don't see a problem. For the option: Maybe it should directly be prepared to sort all kinds of qualifiers, even if they are not added in this patch. ================ Comment at: clang/docs/ClangFormatStyleOptions.rst:1378 +**ConstStyle** (``ConstAlignmentStyle``) + Different ways to arrange const. ---------------- davidstone wrote: > steveire wrote: > > klimek wrote: > > > MyDeveloperDay wrote: > > > > aaron.ballman wrote: > > > > > MyDeveloperDay wrote: > > > > > > klimek wrote: > > > > > > > Personally, I'm somewhat against having 3 different aliases for > > > > > > > the options. I'd chose one, even though it doesn't make everybody > > > > > > > happy, and move on. I'm fine with East/West as long as the > > > > > > > documentation makes it clear what it is. > > > > > > If I have to drop the other options, I think I'd want to go with > > > > > > East/West const as I feel it has more momentum, just letting people > > > > > > know before I change the code back (to my original patch ;-) ) > > > > > > > > > > > > https://www.youtube.com/watch?v=gRmI_gsNqcI > > > > > > > > > > > > {F10954065} > > > > > > > > > > > @klimek I requested that we do not go with East/West the options and > > > > > I'm still pretty insistent on it. East/West is a kitschy way to > > > > > phrase it that I think is somewhat US-centric (where we make a pretty > > > > > big distinction between the east and west coasts). I do not want to > > > > > have to mentally map left/right to the less-clear east/west in the > > > > > config file. Would you be fine if we only had Left/Right instead of > > > > > East/West? I would be fine with that option, but figured enough > > > > > people like the cute East/West designation that we might as well > > > > > support it. > > > > Just for a reference, I'm not from the US and I think east/west still > > > > translates pretty well. I was happy to support the others. > > > I'd be fine with only having left/right; my personal feeling is also that > > > west-const / east-const has kinda become a term of art, though, so I > > > really don't know which one is "right" :) > > > > > > Generally, I think this is one of the cases where, given good docs, we're > > > quickly spending more engineering hours discussing the right solution > > > than it'll cost aggregated over all future users, under the assumption > > > that people do not write new configs very often, and the few who will, > > > will quickly remember. > > > > > > I'd be fine with only having left/right; my personal feeling is also that > > > west-const / east-const has kinda become a term of art, though, so I > > > really don't know which one is "right" :) > > > > This reminds me of the joke that Americans drive on the "Right" side of the > > road, and English drive on the "Correct" side. Sort of gives a different > > meaning to `ConstStyle : Right` and that the alternative is `Wrong` :). > > Maybe that language ambiguity is why `East`/`West` caught on. > > > > > people do not write new configs very often > > > > Agreed. It seems a small number of strong views might influence this enough > > to make `East`/`West` not be used. What a pity. > I'd also add that the original "joke" was `const west` vs `east const` -- the > terms put the const where they would apply in the style. This seems to have > gotten lost in the retelling and now the const is always on the right. > > I'd favor `Left` and `Right`. `East` and `West` have "caught on" now, but > it's caught on among the small group of C++ enthusiasts and still has a sort > of ingroup joke feel to it. I instinctively know my preferred style is "east" > because the phrase "east const" feels more natural to me because I've said it > more. It took me a bit of repetition of thinking "East is right, I put the > const on the right" to get there. > > `East` and `West` also sets a precedent for future options to refer to > positions as `North` and `South`. I also think `Left` and `Right` are the better choices, but I would be full in favor of also parsing `West` and `East`. :) ================ Comment at: clang/lib/Format/EastWestConstFixer.cpp:293 + Tok->Next->Next->Next && Tok->Next->Next->Next->is(tok::kw_const)) { + swapFourTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next, Tok->Next->Next->Next, + /*West=*/true); ---------------- aaron.ballman wrote: > curdeius wrote: > > MyDeveloperDay wrote: > > > rsmith wrote: > > > > There can be more than four type-specifiers / cv-qualifiers in a row. > > > > Eg: `unsigned long long int volatile const` -> `const volatile unsigned > > > > long long int`. > > > you have the volatile moving too? if you had the choice would it become: > > > > > > - `const unsigned long long int volatile` > > > - `const volatile unsigned long long int` > > > - `volatile const unsigned long long int` > > > > > > Any reason why? or is that personal taste? what would be the ideal? > > > > > > > > Given the size of this revision, it would be probably wiser not to touch > > anything else than `const`. > > Any reason why? or is that personal taste? what would be the ideal? > > It's a bit odd to only handle one qualifier and it's been my experience that > people usually want their qualifiers grouped together. While I'm sure there > are opinions on the ordering *within* the qualifiers where there are multiple > different ones, I would guess that there's not a lot of people who would > argue that they want their const qualifiers on the left and their volatile > qualifiers on the right. > > Btw, don't forget that attributes also need to be handled properly. e.g., > ``` > long static constexpr unsigned __attribute__((address_space(0))) long const > int i = 12; > ``` > Also, we probably need a test case where the qualifiers have been duplicated, > like in this lovely `const` fort: > ``` > const const const > const int const > const const const i = 12; > ``` > Do we want that to produce `const const const const const const const const > int i = 12;` or `const int i = 12;`? I would be in favor of just `const int`. ================ Comment at: clang/lib/Format/Format.cpp:1260 GoogleStyle.CommentPragmas = "(taze:|^/[ \t]*<|tslint:|@see)"; + GoogleStyle.ConstPlacement = FormatStyle::CS_Leave; // TODO: enable once decided, in particular re disabling bin packing. ---------------- These are not needed if the LLVM style is already `Leave`. 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
