aaron.ballman added a comment. In D69764#1732295 <https://reviews.llvm.org/D69764#1732295>, @MyDeveloperDay wrote:
> In D69764#1732235 <https://reviews.llvm.org/D69764#1732235>, @aaron.ballman > wrote: > > > I like the functionality, but am slightly opposed to using "east/west" > > terminology -- that's not a ubiquitous phrase and it takes a bit of > > thinking before it makes sense. I think "left/right" is likely to be more > > universally understood. > > > > Also, should this apply to other qualifiers like `volatile` or `restrict`? > > If so, the name `ConstStyle` should probably be `CVQualifierStyle` or > > something else. > > > East/West I think is a term coined I think by Simon Brand (@TartanLlama > https://twitter.com/tartanllama) or maybe Dan Saks before, I did a quick > check on github and both "west const","east const" ,"left const" and "right > const" are all used in commit messages, I'd really like to keep the East/West > just because that's the name of the rebellion (not affiliated!), but we could > alias to `Left` and `Right` (like we sometimes do for true/false when we > overload the options), I've also seen const `Before` and const `After` used. > again if people don't think its too much overload I'd be happy to add those > if it means people get the gist a little clearer. I think east/west is more of a C++ community term than a C community one (and it's a newer phrase). I'd be fine with before/after as well as left/right, with an alias for east/west, but I'd like the canonical name to not be east/west. > I'd probably agree with the CVQualifierStyle although not such a nice > command-line name, what also concerns me is sometimes you see `const volatile > type` and sometimes `volatile const type` sometimes `type const volatile` > almost feels like volatile might needs its own VolatileStyle allowing > > const volatile int (ConstStyle: West, VolatileStyle: East) > volatile const int (ConstStyle: West, VolatileStyle: West) > int const volatile (ConstStyle: East, VolatileStyle: East) > int volatile const (ConstStyle: West, VolatileStyle: West) > > > Whilst I agree one option would be better, I think we'll end up being asked > for more flexible support, such is the way for people to want their own style. If we wanted to add more flexible support, we could do so with a placeholder syntax, like `CVQualifierStyle: volatile const %ident% restrict` or some such, so I don't think such a name fully closes the design space. However, if we do want to stick with separate styles for each of these, we should have explicit comments and tests showing this should only work for `const` qualifiers. 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
