[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 inline comments.


================
Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:65-66
                        has(varDecl(hasLocalStorage(),
-                                   hasType(matchers::isExpensiveToCopy()),
+                                   hasType(hasCanonicalType(
+                                       allOf(matchers::isExpensiveToCopy(),
+                                             unless(hasDeclaration(namedDecl(
----------------
lebedev.ri wrote:
> Does it matter whether we are calling `matchers::isExpensiveToCopy()` on the type, or on the canonical type?
the canonical type does resolve all typedefs, which is what is desirable in this case.


================
Comment at: clang-tidy/utils/Matchers.h:51
 
+AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector<std::string>,
+              NameList) {
----------------
lebedev.ri wrote:
> Please double-check that this does not result in a copy, and `std::vector` is passed as a reference.
its const&, see clang/include/clang/ASTMatchers/ASTMatchersMacros.h line 139


================
Comment at: clang-tidy/utils/Matchers.h:53
+              NameList) {
+  for (const std::string &Name: NameList) {
+    if (llvm::Regex(Name).match(Node.getName()))
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > ```
> > for (const std::string &Name : NameList) {
> > ```
> Do we want to be explicit about early-return here?
it looks like a `return llvm::any_of(NameList, []() { /* Match regex */});`


https://reviews.llvm.org/D52727