hokein added inline comments.
================ Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:39 + auto WhitelistClassMatcher = + cxxRecordDecl(hasAnyName(SmallVector<StringRef, 4>( + WhitelistClasses.begin(), WhitelistClasses.end()))); ---------------- JonasToth wrote: > I have seens this pattern now multiple times in various checks, we should > have some utility wrapping the `hasAnyName(MyList)`. (Just in general, does > not need to happen with this check) no needed for this patch. But yes! Moving to utility is reasonable to me. ================ Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:93 return false; if (utils::ExprMutationAnalyzer(ForRange.getBody(), &Context) .isMutated(&LoopVar)) ---------------- JonasToth wrote: > Adding a `..IsMutated(&LoopVar) && IsReferenced(ForScope, LoopVar))` here > should fix the warning as well. I think ignoring `for (auto _ : state)` is a sensible solution. Thanks! ================ Comment at: test/clang-tidy/performance-for-range-copy.cpp:1 -// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -- -std=c++11 -fno-delayed-template-parsing +// RUN: %check_clang_tidy %s performance-for-range-copy %t -config="{CheckOptions: [{key: "performance-for-range-copy.WhitelistClasses", value: "WhitelistFoo"}]}" -- -std=c++11 -fno-delayed-template-parsing ---------------- JonasToth wrote: > I would prefer a single file, that has the configuration and leave the > standard test like it is. > > with this, you can always track what is actually done by default and what is > done with different conigurations + potential inconsistencies that might > occur by bugs in the configured code. no needed this configuration now. 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