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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Given it is about the performance in loops and the optimizer?! can
better work with the copy-and-don't-use it might make sense to just
check if the variable is dereferenced in the scope of the loop (any
declRefExpr exists).

That is more userfriendly, too.

Am 08.08.2018 um 17:02 schrieb Haojian Wu via Phabricator:

> 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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50447