hokein added a comment.

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
  }


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

Reply via email to