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

[courbet]

Summary

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

[courbet]

fix release note placemenfix release note placement

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

391

Please use double back-ticks for const &&.

clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp

118

Is it really needed?

[courbet]

address review comments

[aaron.ballman]

The release note references documentation that doesn't exist, so Sphinx is failing: https://lab.llvm.org/buildbot/#/builders/115/builds/46942

[courbet]

In D151092#4368151, @aaron.ballman wrote:

The release note references documentation that doesn't exist, so Sphinx is failing: https://lab.llvm.org/buildbot/#/builders/115/builds/46942

Sorry, fixed in rG62dc3ba8442f

[aaron.ballman]

In D151092#4368225, @courbet wrote:

In D151092#4368151, @aaron.ballman wrote:

The release note references documentation that doesn't exist, so Sphinx is failing: https://lab.llvm.org/buildbot/#/builders/115/builds/46942

Sorry, fixed in rG62dc3ba8442f

Thanks! In retrospect, I should have fixed that myself rather than mention it here, sorry for that. :-D

clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp

void NoAutomaticMoveCheck::registerMatchers(MatchFinder *Finder) {
   // `DstT::DstT(Src&&)`.
   const auto LValueRefCtor = cxxConstructorDecl(
       hasParameter(0,
                    hasType(lValueReferenceType(pointee(type().bind("SrcT"))))),
       ofClass(cxxRecordDecl(hasMethod(cxxConstructorDecl(
           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,
+                      hasDeclaration(anyOf(LValueRefCtor, ConstRefRefCtor)),
       this);
 }
 
 void NoAutomaticMoveCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Var = Result.Nodes.getNodeAs<VarDecl>("vardecl");
   const auto *CtorCall = Result.Nodes.getNodeAs<Expr>("ctor_call");
   diag(CtorCall->getExprLoc(), "constness of '%0' prevents automatic move")
       << Var->getName();
 }
 
 void NoAutomaticMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AllowedTypes",
                 utils::options::serializeStringList(AllowedTypes));
 }
 
 } // namespace clang::tidy::performance

clang-tools-extra/docs/ReleaseNotes.rst

- Improved :doc:`modernize-concat-nested-namespaces
   <clang-tidy/checks/modernize/concat-nested-namespaces>` to fix incorrect fixes when
   using macro between namespace declarations, to fix false positive when using namespace
   with attributes and to support nested inline namespace introduced in c++20.
 
 - Fixed a false positive in :doc:`performance-no-automatic-move
   <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
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/performance/no-automatic-move.cpp

 // RUN: %check_clang_tidy -std=c++11-or-later %s performance-no-automatic-move %t
 
 struct Obj {
   Obj();
   Obj(const Obj &);
   Obj(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) {
   const Obj obj1;
   const Obj obj2;
   if (b) {
     return obj1;
     // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: constness of 'obj1' prevents automatic move [performance-no-automatic-move]
   }
   return obj2;
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: constness of 'obj2' prevents automatic move [performance-no-automatic-move]
 }
 
 // FIXME: Ideally we would warn here too.
 NonTemplate PositiveNonTemplateLifetimeExtension() {
   const Obj &obj = Make<Obj>();
   return obj;
 }
 
 // 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;
 }
 
 Obj ConstNrvo() {
   const Obj obj = Make<Obj>();
   return obj;
 }
 
 Obj NotNrvo(bool b) {
   Obj obj1;
   Obj obj2;
   if (b) {
     return obj1;
   }
   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 &);
 };
 
 FromConstRefOnly FromConstRefOnly() {
   const Obj obj = Make<Obj>();
   return obj;
 }