[clang-tools-extra] 9182c67 - [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

[Clement Courbet via cfe-commits]

Author: Clement Courbet
Date: 2023-05-24T15:05:39+02:00
New Revision: 9182c679dde7cb6480e66b9231a53d43ad03908b

URL: https://github.com/llvm/llvm-project/commit/9182c679dde7cb6480e66b9231a53d43ad03908b
DIFF: https://github.com/llvm/llvm-project/commit/9182c679dde7cb6480e66b9231a53d43ad03908b.diff

LOG: [clang-tidy]performance-no-automatic-move: fix false negative on `const T&&` ctors.

We were only handling `const T&`/`T&&` ctor pairs, and we were missing uref-based ctors.

Differential Revision: https://reviews.llvm.org/D151092

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
index 42bf8ff831868..7022e9d784fa7 100644
--- a/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp
@@ -48,15 +48,23 @@ void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) {
           hasParameter(0, hasType(rValueReferenceType(
                               pointee(type(equalsBoundNode("SrcT")))))))))));
 
+  // A matcher for `DstT::DstT(const Src&&)`, which typically comes from an
+  // instantiation of `template <typename U> DstT::DstT(U&&)`.
+  const auto ConstRefRefCtor = cxxConstructorDecl(
+      parameterCountIs(1),
+      hasParameter(0,
+                   hasType(rValueReferenceType(pointee(isConstQualified())))));
+
   Finder->addMatcher(
-      traverse(TK_AsIs,
-               returnStmt(hasReturnValue(
-                   ignoringElidableConstructorCall(ignoringParenImpCasts(
-                       cxxConstructExpr(
-                           hasDeclaration(LValueRefCtor),
-                           hasArgument(0, ignoringParenImpCasts(declRefExpr(
-                                              to(NonNrvoConstLocalVariable)))))
-                           .bind("ctor_call")))))),
+      traverse(
+          TK_AsIs,
+          returnStmt(hasReturnValue(
+              ignoringElidableConstructorCall(ignoringParenImpCasts(
+                  cxxConstructExpr(
+                      hasDeclaration(anyOf(LValueRefCtor, ConstRefRefCtor)),
+                      hasArgument(0, ignoringParenImpCasts(declRefExpr(
+                                         to(NonNrvoConstLocalVariable)))))
+                      .bind("ctor_call")))))),
       this);
 }
 

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 7fe2b97de8643..d1fd47542bd51 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -387,6 +387,10 @@ Changes in existing checks
   <clang-tidy/checks/performance/no-automatic-move>` when warning would be
   emitted for a const local variable to which NRVO is applied.
 
+- Improved :doc:`performance-no-automatic-move
+  <clang-tidy/checks/performance/performance-no-automatic-move>`: warn on ``const &&``
+  constructors.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
index d365f7de8b7c1..1ef7d16e38393 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp
@@ -7,30 +7,42 @@ struct Obj {
   virtual ~Obj();
 };
 
-template <typename T>
-struct StatusOr {
-  StatusOr(const T &);
-  StatusOr(T &&);
-};
-
 struct NonTemplate {
   NonTemplate(const Obj &);
   NonTemplate(Obj &&);
 };
 
+template <typename T> struct TemplateCtorPair {
+  TemplateCtorPair(const T &);
+  TemplateCtorPair(T &&value);
+};
+
+template <typename T> struct UrefCtor {
+  template <class U = T> UrefCtor(U &&value);
+};
+
 template <typename T>
 T Make();
 
-StatusOr<Obj> PositiveStatusOrConstValue() {
+NonTemplate PositiveNonTemplate() {
   const Obj obj = Make<Obj>();
-  return obj;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+  return obj; // selects `NonTemplate(const Obj&)`
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents
+  // automatic move [performance-no-automatic-move]
 }
 
-NonTemplate PositiveNonTemplateConstValue() {
+TemplateCtorPair<Obj> PositiveTemplateCtorPair() {
   const Obj obj = Make<Obj>();
-  return obj;
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents automatic move [performance-no-automatic-move]
+  return obj; // selects `TemplateCtorPair(const T&)`
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents
+  // automatic move [performance-no-automatic-move]
+}
+
+UrefCtor<Obj> PositiveUrefCtor() {
+  const Obj obj = Make<Obj>();
+  return obj; // selects `UrefCtor(const T&&)`
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj' prevents
+  // automatic move [performance-no-automatic-move]
 }
 
 Obj PositiveCantNrvo(bool b) {
@@ -51,22 +63,18 @@ NonTemplate PositiveNonTemplateLifetimeExtension() {
 }
 
 // FIXME: Ideally we would warn here too.
-StatusOr<Obj> PositiveStatusOrLifetimeExtension() {
+UrefCtor<Obj> PositiveUrefCtorLifetimeExtension() {
   const Obj &obj = Make<Obj>();
   return obj;
 }
 
 // Negatives.
 
-StatusOr<Obj> Temporary() {
-  return Make<Obj>();
-}
+UrefCtor<Obj> Temporary() { return Make<Obj>(); }
 
-StatusOr<Obj> ConstTemporary() {
-  return Make<const Obj>();
-}
+UrefCtor<Obj> ConstTemporary() { return Make<const Obj>(); }
 
-StatusOr<Obj> ConvertingMoveConstructor() {
+UrefCtor<Obj> ConvertingMoveConstructor() {
   Obj obj = Make<Obj>();
   return obj;
 }
@@ -85,21 +93,19 @@ Obj NotNrvo(bool b) {
   return obj2;
 }
 
-StatusOr<Obj> Ref() {
+UrefCtor<Obj> Ref() {
   Obj &obj = Make<Obj &>();
   return obj;
 }
 
-StatusOr<Obj> ConstRef() {
+UrefCtor<Obj> ConstRef() {
   const Obj &obj = Make<Obj &>();
   return obj;
 }
 
 const Obj global;
 
-StatusOr<Obj> Global() {
-  return global;
-}
+UrefCtor<Obj> Global() { return global; }
 
 struct FromConstRefOnly {
   FromConstRefOnly(const Obj &);