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
----------------
flx wrote:
> 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.
This is done now.
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
