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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: djasper.
aaron.ballman added a comment.

In D69764#2532413 <https://reviews.llvm.org/D69764#2532413>, @steveire wrote:

> What can be done to move this change along?

Here is my thinking, which is largely unchanged from previous discussion: a code formatting tool should not modify code such that it changes meaning. This sort of behavior causes serious problems in practice, such as difficulty using the formatter in CI pipelines (because the user can't conform to what the formatter wants in some cases) or a lack of trust in the tool when it inserts subtle bugs that don't cause compile errors. I don't think that's acceptable for a production-quality tool that's expected to be run in a (semi-)automated fashion. I don't think the fact that clang-format already has such behavior is a blessing that we should add more such features. Every time we do that, we make a less usable tool for our users. So to me, the acceptance criteria is: the change should not break code, regardless of whether it's behind a flag or not. Flags alleviate some of the pain, certainly, but in clang-format, flags can be shared between multiple users due to the presence of a .clang-format file, and so "just change the configuration" isn't always a trivial bar for everyone to hop over.

I should note that I see the code breaking behavior between #include sort orders changing behavior and the qualifier position code breaking as being slightly different levels of severity. With include order, my code was brittle and the tool exposes that fact. With the qualifier change, my code doesn't necessarily have to be brittle to be broken.

All that said, clang-format is precisely where I'd *expect* this functionality to live, logically, so I agree with the strong desire to land the feature here. But given the option between fast and correct, I want both. If I can't have both, I want correct. So a shorter path forward to surfacing the feature appears to be within clang-tidy where we can use an AST to ensure we get correct behavior (at the expense of performance). There are a lot of longer paths forward, such as exploring why use of an AST is too slow for a code formatting tool and seeing if that can be improved (note, wins here are likely to wind up being wins for compilation speed with Clang overall, which would be very impactful!), but I can understand if the authors don't want to work on that effort. But I don't think "it's hard to be both fast and correct" is a valid reason to add known-problematic functionality to the code formatting tool with no known solution aside from "disable this functionality or find a different tool".


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

https://reviews.llvm.org/D69764