[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69764#2934124 <https://reviews.llvm.org/D69764#2934124>, @klimek wrote:

> In D69764#2934032 <https://reviews.llvm.org/D69764#2934032>, @MyDeveloperDay wrote:
>
>> It was suggested to me that I write up and RFC on the matter, I'm not a massive fan of those as I don't really see a formal process for handling them (the conversation seems to turn into a series of personal preferences then dwindles and dies without conclusion). But if people think it would solve something I guess I could try.
>>
>> From my perspective I'm more interested in the views of the major clang-format contributors (@curdeius, @krasimir, @owenpan , @sammccall, @HazardyKnusperkeks  and people like yourselves who have put time and effort into blogging about these tools), ultimately it will be us who have to handle the impact of this (and maybe the #clangd people) and I don't want to inconvenience them or make work for them.
>>
>> So here is what is needed to land this in my view:
>>
>> 1. I'd want at least 2 to 3 of the current reviewers to LGTM.
>> 2. I'd want 1 person (like yourself) a community contributor to also give their approval (I guess I have that conceptually from you!)
>>
>> or
>>
>> 1. @klimek or @djasper to give us an its ok for "clang-format to insert/modify the order of tokens" in a controlled manner decision
>> 2. 1 LGTM
>>
>> Whilst I respect the views of those few who have objected, they don't to my knowledge make significant contributions to clang-format (not in the last 5-6 years I've been following it anyway)
>>
>> I feel this team owns the decision as we are the ones that look after it, warts and all. We'll always have a situation where some don't agree, thats ok, but I feel there is more agreement that this funcationality is ok rather than not.
>>
>> Until one of those 2 things happens, we are in limbo.
>
> I think we had a really good, inclusive discussion on this change, so I don't think an RFC would add anything to it.

FWIW, the RFC I requested off-list wasn't about east/west const fixing, it was about whether the community was okay with a new direction of clang-format that breaks code or not. This is a shift in direction from the original design of clang-format. The waters were muddied somewhat by include sorting, but in my totally unscientific polls on the matter, I found basically no one concerned about include sorting while I found a not-insignificant number of people who were concerned about qualifier formatting and breakage in general. The prevailing sentiments were that breakage from include sorting demonstrates fragility with your code, but qualifier shuffling may or may not. That said, I did find that about 1/3 of the people were fine with breakage in general so long as it's mandated to be an opt-in feature as a matter of policy (and about 65% were uncomfortable with breakage from a code formatting tool in general). Take that "data" with a heaping grain of salt, it was a Twitter poll with ~150 respondents.

Personally, I still think an RFC (as frustrating of a process as it can be) is valuable because this strikes me as a policy change for a very popular tool. I highly doubt much of the community is following this review with that in mind, let alone the greater user base for clang-format.

> I think we have consensus on, if we want this, we:
>
> 1. want it behind an option that is not enabled in default-configs
> 2. we want users to understand that this has the potential to introduce bugs

+1

> I also think this is a feature that seems useful enough for a large enough group, to provide it in clang-format.
> I'd put a lot of weight on @MyDeveloperDay's evaluation, as in the end, he's currently taking the most active role in keeping clang-format alive & well.
>
> One idea for clang-format going forward is: can we make it easier for people to understand what flags can introduce non-whitespace-changes? E.g. have flag name prefixes like semantic-*.

There are orthogonal ideas here. 1) I think having the flag name clearly show that this may break the user's code is a good idea. 2) I do not think this review should set a new policy that breaking changes in clang-format are acceptable unless there is a larger community discussion specifically about that point.

> So, I'm generally OK with this.

I continue to oppose the decision on the grounds that code formatting tools should not be opinionated on the formatting of their input source code. This reduces the utility of a tool that's used by a *much* wider audience than people represented on this review. That said, I'll abide by the consensus position.


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

https://reviews.llvm.org/D69764