Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -29,6 +29,14 @@ } } +template +bool hasNonDefaultArgs(const Call &CallExpr, unsigned int StartIndex = 0) { + for (unsigned int I = StartIndex; I < CallExpr.getNumArgs(); ++I) + if (!CallExpr.getArg(I)->isDefaultArgument()) + return true; + return false; +} + } // namespace using namespace ::clang::ast_matchers; @@ -54,7 +62,8 @@ on(declRefExpr(to(varDecl().bind("objectArg"))))); auto ConstRefReturningFunctionCall = callExpr(callee(functionDecl(returns(ConstReference))), - unless(callee(cxxMethodDecl()))); + unless(callee(cxxMethodDecl()))) + .bind("initFunctionCall"); auto localVarCopiedFrom = [this](const internal::Matcher &CopyCtorArg) { return compoundStmt( @@ -96,6 +105,8 @@ const auto *ObjectArg = Result.Nodes.getNodeAs("objectArg"); const auto *BlockStmt = Result.Nodes.getNodeAs("blockStmt"); const auto *CtorCall = Result.Nodes.getNodeAs("ctorCall"); + const auto *InitFunctionCall = + Result.Nodes.getNodeAs("initFunctionCall"); TraversalKindScope RAII(*Result.Context, ast_type_traits::TK_AsIs); @@ -108,11 +119,15 @@ // A constructor that looks like T(const T& t, bool arg = false) counts as a // copy only when it is called with default arguments for the arguments after // the first. - for (unsigned int i = 1; i < CtorCall->getNumArgs(); ++i) - if (!CtorCall->getArg(i)->isDefaultArgument()) - return; + if (hasNonDefaultArgs(*CtorCall, 1)) + return; if (OldVar == nullptr) { + // Only allow initialization of a const reference from a free function if it + // has no arguments. Otherwise it could return an alias to one of its + // arguments and the arguments need to be checked for const use as well. + if (InitFunctionCall != nullptr && InitFunctionCall->getNumArgs() > 0) + return; handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, ObjectArg, *Result.Context); } else { @@ -127,10 +142,10 @@ bool IsConstQualified = Var.getType().isConstQualified(); if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context)) return; + if (ObjectArg != nullptr && !isOnlyUsedAsConst(*ObjectArg, BlockStmt, Context)) return; - auto Diagnostic = diag(Var.getLocation(), IsConstQualified ? "the const qualified variable %0 is " Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp +++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp @@ -23,6 +23,9 @@ ExpensiveToCopyType global_expensive_to_copy_type; const ExpensiveToCopyType &ExpensiveTypeReference(); +const ExpensiveToCopyType &freeFunctionWithArg(const ExpensiveToCopyType &); +const ExpensiveToCopyType &freeFunctionWithDefaultArg( + const ExpensiveToCopyType *arg = nullptr); const TrivialToCopyType &TrivialTypeReference(); void mutate(ExpensiveToCopyType &); @@ -387,3 +390,18 @@ for (const Element &E : Container()) { } } + +// This should not trigger the check as the argument could introduce an alias. +void negativeInitializedFromFreeFunctionWithArg() { + ExpensiveToCopyType Orig; + const ExpensiveToCopyType Copy = freeFunctionWithArg(Orig); +} + +void negativeInitializedFromFreeFunctionWithDefaultArg() { + const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(); +} + +void negativeInitialzedFromFreeFunctionWithNonDefaultArg() { + ExpensiveToCopyType Orig; + const ExpensiveToCopyType Copy = freeFunctionWithDefaultArg(&Orig); +}