[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

[Felix Berger via Phabricator via cfe-commits]

flx added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:98-101
   auto Matches =
       match(findAll(declStmt(has(varDecl(equalsNode(&InitializingVar))))
                         .bind("declStmt")),
+            Body, Context);
----------------
ymandel wrote:
> Consider inspecting the `DeclContext`s instead, which should be much more efficient than searching the entire block.  Pass the `FunctionDecl` as an argument instead of `Body`, since it is a `DeclContext`.  e.g. `const DeclContext &Fun`
> 
> Then, either
> 1. Call `Fun.containsDecl(InitializingVar)`, or
> 2. Search through the contexts yourself; something like:
> ```
> DeclContext* DC = InitializingVar->getDeclContext(); 
> while (DC != nullptr && DC != &Fun)
>   DC = DC->getLexicalParent();
> if (DC == nullptr)
>   // The reference or pointer is not initialized anywhere witin the function. We
>   // assume its pointee is not modified then.
>   return true;
> ```
Are #1 and #2 equivalent? From the implementation and comment I cannot tell whether #1 would cover cases where the variable is not declared directly in the function, but in child block:

```
void Fun() {
 {
   var i;
   {
     i.usedHere();
   }  
 } 
}
```

I'm also reading this as an optimization to more quickly determine whether we can stop here. We still need to find the matches for the next steps, but I  think I could then limit matching to the DeclContext I found here. Is this correct? For this I would actually need the DeclContext result from #2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103021