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

[Felix Berger via Phabricator via cfe-commits]

flx updated this revision to Diff 291610.
flx marked an inline comment as done.

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

https://reviews.llvm.org/D87455

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


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);
+}
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
@@ -54,7 +54,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<Expr> &CopyCtorArg) {
     return compoundStmt(
@@ -96,6 +97,8 @@
   const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>("objectArg");
   const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+  const auto *InitFunctionCall =
+      Result.Nodes.getNodeAs<CallExpr>("initFunctionCall");
 
   TraversalKindScope RAII(*Result.Context, ast_type_traits::TK_AsIs);
 
@@ -113,6 +116,11 @@
       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 {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87455.291610.patch
Type: text/x-patch
Size: 3043 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200914/2bf7adc2/attachment.bin>