lebedev.ri added a comment.
That's it!
Hopefully last portion of nits.
================
Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:65-66
has(varDecl(hasLocalStorage(),
- hasType(matchers::isExpensiveToCopy()),
+ hasType(hasCanonicalType(
+ allOf(matchers::isExpensiveToCopy(),
+ unless(hasDeclaration(namedDecl(
----------------
Does it matter whether we are calling `matchers::isExpensiveToCopy()` on the
type, or on the canonical type?
================
Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:83-86
+ unless(anyOf(referenceType(),
+ hasDeclaration(namedDecl(
+ matchers::matchesAnyListedName(AllowedTypes))))),
+ matchers::isExpensiveToCopy()))),
----------------
This is inconsistent with the rest of the matchers here.
I think you want to check `isExpensiveToCopy()` first, because that should be
less expensive than regex.
================
Comment at: clang-tidy/utils/Matchers.h:51
+AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector<std::string>,
+ NameList) {
----------------
Please double-check that this does not result in a copy, and `std::vector` is
passed as a reference.
================
Comment at: clang-tidy/utils/Matchers.h:53
+ NameList) {
+ for (const std::string &Name: NameList) {
+ if (llvm::Regex(Name).match(Node.getName()))
----------------
```
for (const std::string &Name : NameList) {
```
================
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:
> ```
> for (const std::string &Name : NameList) {
> ```
Do we want to be explicit about early-return here?
https://reviews.llvm.org/D52727
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
