[PATCH] D50447: [clang-tidy] Add a whitelistClasses option in performance-for-range-copy check.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a subscriber: EricWF.
lebedev.ri added a comment.

In https://reviews.llvm.org/D50447#1192316, @hokein wrote:

> In https://reviews.llvm.org/D50447#1192280, @JonasToth wrote:
>
> > If whitelisting works, thats fine. But I agree with @lebedev.ri that a contribution/improvement to the ExprMutAnalyzer is the better option. This is especially the case, because multiple checks (and maybe even some other parts then clang-tidy) will utilize this analysis.
>
>
> I'm sorry for not explaining it with more details. Might be "regression" is a confusing world :(. It is not false positive. The change using ExprMutAnalyzer is reasonable, IMO. It makes this check smarter to catch cases which will not be caught before. For example,
>
>   for (auto widget : container) {
>     widget.call_const_method(); // const usage recongized by ExprMutAnalyzer, so it is fine to change widget to `const auto&`
>   } 
>
>
> But in our codebase, we have code intended to use like below, and it is in base library, and is used widely. I think it makes sense to whitelist this class in our internal configuration.
>
>   for (auto _ : state) {
>      ... // no `_` being referenced in the for-loop body
>   }
>


That looks remarkably like google benchmark main loop. (i don't know at hand if making this const will break it, but probably not?)
I wonder if instead there should be an option to not complain about the variables that aren't actually used?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50447