[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 15 05:16:03 PDT 2022


aaron.ballman added a comment.

In D131386#3722849 <https://reviews.llvm.org/D131386#3722849>, @aaron.ballman wrote:

> In D131386#3722822 <https://reviews.llvm.org/D131386#3722822>, @njames93 wrote:
>
>> In D131386#3722749 <https://reviews.llvm.org/D131386#3722749>, @aaron.ballman wrote:
>>
>>> We leave formatting decisions in clang-tidy to clang-format and I don't think we should deviate from that policy here without a very clear understanding of when we should relax that restriction. That said, I'm personally not certain we should have such an option (the long-term goal has generally been to integrate clang-format functionality into clang-tidy so there can be an option to just run format after applying fixes in a TU). Is there a compelling reason we should have it?
>>
>> The reason for this is due to the issue that `QualifierAlignment`  is a non whitespace only change and clang-format lists that using it could break some code.
>
> Yes, and the community decided that risk was reasonable for clang-format.
>
>> In light of this some users may wish to set the option to `QAS_Leave` to be sure no code is broken even though they would prefer a specific style. 
>> Therefore having a dedicated option in the check will let those users specify the style, without having to set a clang-format configuration which they aren't content in using.
>
> When we decided that clang-format was allowed to break code, we also decided that any time it does so on real world code is considered a bug and is something we should work to fix. So I'm not in favor of this change; if clang-tidy uncovers a bug in clang-format, that should be fixed in clang-format rather than worked around in clang-tidy. And if clang-tidy can't trust clang-format to be production quality, that's something we should address with the clang-format folks as a serious concern that needs a plan of action. But I don't think introducing formatting options into clang-tidy is a road we want to go down.

I suppose another way to look at it is: clang-format options that are considered dangerous (comes with a warning as QualifierAlignment does in https://clang.llvm.org/docs/ClangFormatStyleOptions.html) are things clang-tidy checks with fixes could have options for. However, that gets awkward when clang-format finds a way to remove that warning; then clang-tidy is left supporting formatting options that it shouldn't have (but removing those options can break people's .clang-tidy config files), so I'm not certain I like that approach any better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386



More information about the cfe-commits mailing list