[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

True

Am 02.10.2018 um 22:28 schrieb Roman Lebedev via Phabricator:

> lebedev.ri added inline comments.
> 
> ================
>  Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:50
>  +  const auto VarType = Var->getType();
>  +  if (std::find_if(WhiteListTypes.begin(), WhiteListTypes.end(),
>  +                   [&](llvm::StringRef WhiteListType) {
> 
>  ----------------
> 
> JonasToth wrote:
> 
>> baloghadamsoftware wrote:
>> 
>>> JonasToth wrote:
>>> 
>>>> lebedev.ri wrote:
>>>> 
>>>>> `llvm::any_of()`
>>>> 
>>>> This configuration should be used in the matchers. Please see `cppcoreguidelines-no-malloc` for an example on how to do it in the matchers. Having it there usually improves performance and is clearer.
>>> 
>>> `google-runtime-references` has this in the `check()` function. It seems there is no common agreement where to put this. `cppcoreguidelines-no-malloc` is not a good example since it simple compares strings instead of matching regular expressions. I think here we should allow regular expressions. Then we would need a new matcher called `matchesAnyName`. Should I create it?
>> 
>> Yes, regex is not supported there, but the mechanism is the same. You can put the new matcher in this check for now, if it's generally usefully in other clang-tidy check we can easily migrate them later on.
>> 
>> My motiviation for wanting it in the matcher instead of the check is to reduce the amount of ineffective work. If the matcher already ignores these cases, we don't need to spend cycles doing the callbacks and all the machinery. Its not a strong opinion though, if you don't agree I am fine with it.
>> 
>> Do you want to provide many matchers? In principle one could make one big regex with (`PATTERN1|PATTERN2|PATTERN3`). What do you think?
>>  You can put the new matcher in this check for now, if it's generally usefully in other clang-tidy check we can easily migrate them later on.
> 
> Since it's now used in 4 places, i think it should be `clang-tidy/utils/Matchers.h`
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D52727


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52727