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