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
