[PATCH] D87455: [clang-tidy] performance-unnecessary-copy-initialization: Restrict UnnecessaryCopyInitialization check to variables initialized from free functions without arguments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:126
   if (OldVar == nullptr) {
+    // Only allow initialization of a const reference from a free function if it
+    // has no arguments or only default arguments. Otherwise it could return
----------------
I'm not certain I agree with this change, but I'm also not certain I disagree with it. There are more ways to alias than just default arguments (global variables, etc) and the presence of a default argument is not really an indication about its return value in this case. e.g.,
```
const ExpensiveToCopyType &freeFunctionWithDefaultArg(int i = 12);
```
(I don't see a reason why `= 12` should impact whether use of this function diagnoses or not.)

Perhaps if this was tightened up a bit to care about the type of the default argument it might make sense, but even that gets tricky when you consider base classes, template types, etc. and doesn't cover all of the aliasing cases anyway. Is there an important use case for this behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87455