[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

Mon Oct 19 06:52:13 PDT 2020

aaron.ballman added a comment.

In D89332#2338319 <https://reviews.llvm.org/D89332#2338319>, @njames93 wrote:

> Come to think of it, this is a pretty illogical way to solve this problem, just append `::std::function` to the AllowedTypes vector in `registerMatchers` and be do with it. Will require dropping the const Qualifier on AllowedTypes, but aside from that it is a much simpler fix.
> The has(Any)Name matcher has logic for skipping inline namespaces.

This also seems like a reasonable alternative, to me.

================
Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:39
+// is unnecessary.
+AST_MATCHER(NamedDecl, isStdFunction) {
+  // First use the fast getName() method to avoid unnecessary calls to the
----------------
njames93 wrote:
> It's better to use `node.isInStdNamespace()` instead of checking the qualified name as its designed for that very purpose. Is should probably take a `CXXRecordDecl` instead of a `NamedDecl` aswell.
There's no need for this matcher -- `hasName("::std::function")` is the correct way to test for this.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89332/new/

https://reviews.llvm.org/D89332