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

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 14 05:47:48 PDT 2021


curdeius added a comment.

I've been trying to make my opinion on this patch for the last few weeks...
I was pretty much opposed to introducing non-whitespace chances previously, but I'm reviewing my standpoint.
As mentioned already, there are precedents (include sorting, namespace comments, long string splitting).
I'm however even more wary of adding yet another tool that will be almost the same as clang-format. It could work if it were a drop-in replacement of clang-format, but that seems to be very much wishful thinking to me.
First, maintenance burden to verify that the two don't diverge somehow. Secondly, the drop-in replacement wouldn't be possible without changing clang-format itself (e.g. to ignore style options that are for "clang-format++" only). Also, it might divide the users into clang-format camp and clang-format++ camp (which may or may not be a problem).
Lastly, I do think that clang-format can be as reliable with this patch as it's now. Breaking code is of course possible but that's the case of many style options. And these are bugs that will eventually get fixed. It's of course important that this option doesn't break anything ever by default, but given that the default is Leave, and it's implemented as an additional pass, that should be the case.
Also, I'd be a bit surprised if people used it in CI immediately after this feature has landed without verifying that it doesn't break anything on their codebase.

On the other hand, clang-tidy has a corresponding check. I do feel though that's a sort of heavyweight tool and much less used than clang-format. Also, the placing of const qualifier is by many (at least in my circles) associated to the latter tool.

So yes, I'm in favour of landing this patch (though not exactly in the current form, I'd prefer more future-proof options for instance, not only handling const).
My (longish) 2 cents.


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

https://reviews.llvm.org/D69764



More information about the cfe-commits mailing list