[clang-tidy] Add readability-reference-to-constructed-temporary check

Detects code where a temporary object is directly constructed by calling
a constructor or using an initializer list and immediately assigned to a
reference variable.

Reviewed By: xgupta

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

Author: PiotrZSL

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -41,6 +41,7 @@ add_clang_library(clangTidyReadabilityModule
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
+  ReferenceToConstructedTemporaryCheck.cpp
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

@@ -44,6 +44,7 @@
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
+#include "ReferenceToConstructedTemporaryCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
@@ -116,6 +117,8 @@ class ReadabilityModule : public ClangTidyModule {
         "readability-redundant-member-init");
     CheckFactories.registerCheck<RedundantPreprocessorCheck>(
         "readability-redundant-preprocessor");
+    CheckFactories.registerCheck<ReferenceToConstructedTemporaryCheck>(
+        "readability-reference-to-constructed-temporary");
     CheckFactories.registerCheck<SimplifySubscriptExprCheck>(
         "readability-simplify-subscript-expr");
     CheckFactories.registerCheck<StaticAccessedThroughInstanceCheck>(

clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp

@@ -0,0 +1,85 @@
+//===--- ReferenceToConstructedTemporaryCheck.cpp - clang-tidy
+//--------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ReferenceToConstructedTemporaryCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+// Predicate structure to check if lifetime of temporary is not extended by
+// ValueDecl pointed out by ID
+struct NotExtendedByDeclBoundToPredicate {
+  bool operator()(const internal::BoundNodesMap &Nodes) const {
+    const auto *Other = Nodes.getNodeAs<ValueDecl>(ID);
+    if (!Other)
+      return true;
+
+    const auto *Self = Node.get<MaterializeTemporaryExpr>();
+    if (!Self)
+      return true;
+
+    return Self->getExtendingDecl() != Other;
+  }
+
+  StringRef ID;
+  ::clang::DynTypedNode Node;
+};
+
+AST_MATCHER_P(MaterializeTemporaryExpr, isExtendedByDeclBoundTo, StringRef,
+              ID) {
+  NotExtendedByDeclBoundToPredicate Predicate{
+      ID, ::clang::DynTypedNode::create(Node)};
+  return Builder->removeBindings(Predicate);
+}
+
+} // namespace
+
+bool ReferenceToConstructedTemporaryCheck::isLanguageVersionSupported(
+    const LangOptions &LangOpts) const {
+  return LangOpts.CPlusPlus;
+}
+
+std::optional<TraversalKind>
+ReferenceToConstructedTemporaryCheck::getCheckTraversalKind() const {
+  return TK_AsIs;
+}
+
+void ReferenceToConstructedTemporaryCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(
+      varDecl(unless(isExpansionInSystemHeader()),
+              hasType(qualType(references(qualType().bind("type")))),
+              decl().bind("var"),
+              hasInitializer(expr(hasDescendant(
+                  materializeTemporaryExpr(
+                      isExtendedByDeclBoundTo("var"),
+                      has(expr(anyOf(cxxTemporaryObjectExpr(), initListExpr(),
+                                     cxxConstructExpr()),
+                               hasType(qualType(equalsBoundNode("type"))))))
+                      .bind("temporary"))))),
+      this);
+}
+
+void ReferenceToConstructedTemporaryCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<VarDecl>("var");
+  const auto *MatchedTemporary = Result.Nodes.getNodeAs<Expr>("temporary");
+
+  diag(MatchedDecl->getLocation(),
+       "reference variable %0 extends the lifetime of a just-constructed "
+       "temporary object %1, consider changing reference to value")
+      << MatchedDecl << MatchedTemporary->getType();
+}
+
+} // namespace clang::tidy::readability

clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h

@@ -0,0 +1,34 @@
+//===--- ReferenceToConstructedTemporaryCheck.h - clang-tidy ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REFERENCETOCONSTRUCTEDTEMPORARYCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REFERENCETOCONSTRUCTEDTEMPORARYCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::readability {
+
+/// Detects C++ code where a reference variable is used to extend the lifetime
+/// of a temporary object that has just been constructed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/reference-to-constructed-temporary.html
+class ReferenceToConstructedTemporaryCheck : public ClangTidyCheck {
+public:
+  ReferenceToConstructedTemporaryCheck(StringRef Name,
+                                       ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REFERENCETOCONSTRUCTEDTEMPORARYCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -128,6 +128,12 @@ New checks
   Recommends the smallest possible underlying type for an ``enum`` or ``enum``
   class based on the range of its enumerators.
 
+- New :doc:`readability-reference-to-constructed-temporary
+  <clang-tidy/checks/readability/reference-to-constructed-temporary>` check.
+
+  Detects C++ code where a reference variable is used to extend the lifetime
+  of a temporary object that has just been constructed.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -380,6 +380,7 @@ Clang-Tidy Checks
    `readability-redundant-smartptr-get <readability/redundant-smartptr-get.html>`_, "Yes"
    `readability-redundant-string-cstr <readability/redundant-string-cstr.html>`_, "Yes"
    `readability-redundant-string-init <readability/redundant-string-init.html>`_, "Yes"
+   `readability-reference-to-constructed-temporary <readability/reference-to-constructed-temporary.html>`_,
    `readability-simplify-boolean-expr <readability/simplify-boolean-expr.html>`_, "Yes"
    `readability-simplify-subscript-expr <readability/simplify-subscript-expr.html>`_, "Yes"
    `readability-static-accessed-through-instance <readability/static-accessed-through-instance.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst

@@ -0,0 +1,32 @@
+.. title:: clang-tidy - readability-reference-to-constructed-temporary
+
+readability-reference-to-constructed-temporary
+==============================================
+
+Detects C++ code where a reference variable is used to extend the lifetime of
+a temporary object that has just been constructed.
+
+This construction is often the result of multiple code refactorings or a lack
+of developer knowledge, leading to confusion or subtle bugs. In most cases,
+what the developer really wanted to do is create a new variable rather than
+extending the lifetime of a temporary object.
+
+Examples of problematic code include:
+
+.. code-block:: c++
+
+   const std::string& str("hello");
+
+   struct Point { int x; int y; };
+   const Point& p = { 1, 2 };
+
+In the first example, a ``const std::string&`` reference variable ``str`` is
+assigned a temporary object created by the ``std::string("hello")``
+constructor. In the second example, a ``const Point&`` reference variable ``p``
+is assigned an object that is constructed from an initializer list ``{ 1, 2 }``.
+Both of these examples extend the lifetime of the temporary object to the
+lifetime of the reference variable, which can make it difficult to reason about
+and may lead to subtle bugs or misunderstanding.
+
+To avoid these issues, it is recommended to change the reference variable to a
+(``const``) value variable.

clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp

@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-reference-to-constructed-temporary %t
+
+struct WithConstructor
+{
+    WithConstructor(int, int);
+};
+
+struct WithoutConstructor
+{
+    int a, b;
+};
+
+void test()
+{
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: reference variable 'tmp1' extends the lifetime of a just-constructed temporary object 'const WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithConstructor& tmp1{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: reference variable 'tmp2' extends the lifetime of a just-constructed temporary object 'const WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   const WithoutConstructor& tmp2{1,2};
+
+
+// CHECK-MESSAGES: :[[@LINE+1]]:22: warning: reference variable 'tmp3' extends the lifetime of a just-constructed temporary object 'WithConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithConstructor&& tmp3{1,2};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: reference variable 'tmp4' extends the lifetime of a just-constructed temporary object 'WithoutConstructor', consider changing reference to value [readability-reference-to-constructed-temporary]
+   WithoutConstructor&& tmp4{1,2};
+
+   WithConstructor tmp5{1,2};
+   WithoutConstructor tmp6{1,2};
+}