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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> 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?

Yeah, it is google benchmark library.

The new fix `for (const auto& _ : state)` will trigger -Wunused warning. `__attribute__((unused))` doesn't help to suppress the warning :(

  $ cat /tmp/main4.cc 
  #include <vector>
  
  struct __attribute__((unused)) Foo {
  };
  
  void f() {
    std::vector<Foo> foos;
    for (const Foo& _ : foos) {
    }
  }
  $ g++ /tmp/main4.cc -Wunused                                                                                                                                                                                 
  /tmp/main4.cc: In function ‘void f()’:
  /tmp/main4.cc:8:19: warning: unused variable ‘_’ [-Wunused-variable]
     for (const Foo& _ : foos) {
                     ^



> Is the type important, or specifics about the variable (e.g. the name?)
>  The _ variable names are sometimes used for RAII variables/lambdas
>  that shall just do some cleanup, e.g. gsl::finally().
> 
> It might make sense to give ExprMutAnalyzer an ignore-mechanism.
> 
> I wonder if instead there should be an option to not complain about the variables that aren't actually used?
> 
> Checking for variable usage would be simple. The ExprMutAnalyzer
>  always analyses the scope, e.g. a function. You can match this scope for
>  a DeclRefExpr of the variable, empty result means no usage.

Both type and variable name "_" can fix the issue here, but the variable name seems a fragile way (if the name is changed, things will be broken again).

Yeah, adding an ignore mechanism to ExprMutAnalyzer is another option.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50447