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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
