flx 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
----------------
aaron.ballman wrote:
> 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?
This was mostly following the logic already applied to the copy constructor
where the existing code also allowed default arguments. The idea is to not
disallow functions with default arguments that are not overridden.
I'll switch to not allowing any arguments. This will just exclude a few more
cases, but better to be safe, until we might be able to verify the constness of
all function arguments as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87455/new/
https://reviews.llvm.org/D87455
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
