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

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 9 07:05:28 PDT 2021


erichkeane added a comment.

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

> In D69764#2934378 <https://reviews.llvm.org/D69764#2934378>, @erichkeane wrote:
>
>> I've just been watching this from the sideline, but the cases where this breaks code are unacceptable for this tool, it is a complete direction change for the tool, and making that direction change silently on a review of a 15 month patch, where TWO code owners have said 'no' for that reason is absurd.
>>
>> I use this tool daily as a part of my 'upload' script, having it silently bust code between when I validate it and when I upload it is terrible, and makes the tool unusable for my purposes.  If we change this direction without a full RFC, my next step is going to be an RFC to remove clang-format from the check-in requirements of the entire LLVM project.
>
> This and other potentially other mutating options would and MUST in my view ALWAYS be 100% "off by default" for all default style options (as -fix is for clang-tidy), it would be a purely "opt in" basis. (via .clang-format or command line)
>
> I personally use this in a non modifying way "using the -dry-run mode" to catch new"east/const violations" and report failure back rather than "change the code itself"
>
> I would not expect clang-format usage to change unless someone specially opted in to using it.

That seems just as bad, if not worse.  Clang-format isn't an analysis tool, its a format tool.  If you have an option that can only reasonably be run in 'dry-run' mode, it seems that putting it in a 'format' tool is the wrong place.


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

https://reviews.llvm.org/D69764



More information about the cfe-commits mailing list