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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to