[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

Mon Aug 1 04:01:31 PDT 2022

aaron.ballman added a comment.

In D54943#3687827 <https://reviews.llvm.org/D54943#3687827>, @JonasToth wrote:

> In D54943#3681527 <https://reviews.llvm.org/D54943#3681527>, @sammccall wrote:
>
>> This check is enabled by default in LLVM (`Checks: misc-*` in `llvm-project/.clang-tidy`)
>>
>> The warning on mutable non-ref local variables is pretty noisy: a *lot* of existing code does not do this, for defensible reasons (some of us think that the ratio of extra safety to extra syntactic noise for locals is too low). The LLVM style guide doesn't take a position on this.
>>
>> Should this check
>>
>> - be disabled for LLVM? (i.e. this is opt-in for codebases with strong const-correctness rules, LLVM does not, it was unintentionally enabled by `misc-*`)
>> - be configured only to warn on references? (i.e. we expect that is de-facto LLVM style and so uncontroversial)
>
> IMHO just enable for references in LLVM, thats still enough warnings to take a while to process and the guide is clear on that.
> If the community wants it for values as well, that can be done later anyway.
>
> Do you think the defaults for this check should change? Originally, it was "default on" because `cppcoreguidelines` state so.

I agree with Sam that this definitely needs to be disabled for LLVM -- we don't have anywhere near enough const correctness to get significant value out of this check yet, even if it was limited to just references. As for whether we should default to only warn on references by default -- I think that might be a better approach now that this is no longer tied directly to the C++ Core Guidelines.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943