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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 13 11:42:23 PDT 2021


aaron.ballman added a comment.

In D69764#2874790 <https://reviews.llvm.org/D69764#2874790>, @MyDeveloperDay wrote:

>> so there's something like precedent here
>
> I think its worth mentioning, that my personal preference would STILL be to land this inside clang-format with default configuration of "OFF", where there is also significant existing precedent for passes that change non whitespace and even add tokens.

Fair.

> I still believe clang-format is the best location, I think its the most optimal, and I think its the fairest (because those that want it get it, and those that don't aren't forced to have it). But doing this as another tool would be a `compromise`, in my view an inferior one to it being in clang-format, but at least we could set out clear goals where allowing code modification was the intent (as this seems to be the major sticking point)
>
> I would be interested to know how many people would be unhappy if we stated that "sorting includes" and "namespace comments" had to be removed from clang-format and into the new tool! I am thinking it would be fairly significant. (I'm not suggesting we would, just making a point)

I would be in favor of such a migration for sorting includes. I want my code formatting tool to not break my code. I recognize the tool already potentially breaks code when reordering fragile includes, but I think that was a mistake that should not be purposefully repeated without the community actively saying that's the direction they want the tool to go in. Personally, I'd be opposed to such a direction (each new breaking transformation supplied by the tool makes the tool that much less trustworthy in common use cases like CI pipelines), but perhaps the community has an appetite for such a change.

(Btw, I don't see how namespace comments really relate -- those don't break code that I'm aware of. If the concern is "but they're not whitespace"; they are as far as the compiler is concerned.)

> So you know, I'm pushing because I'm being ask privately to land this, because I assume people want to use it, but maybe don't want to voice their opinions publicly.
>
> I don't want to fragment the community by pushing an agenda (something I see you seem to care about),  but I would also like to think that those of us who contribute to clang-format regularly can help shape a future for it moving forward.

I am not the code owner for this tool, so I think the decision ultimately lands on @rsmith as we have no code owner specifically for this component. However: https://reviews.llvm.org/D69764#2056105 so it's not like I'm alone in my concerns.


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

https://reviews.llvm.org/D69764



More information about the cfe-commits mailing list