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

[PiotrZSL]

Summary

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.

[PiotrZSL]

There are 2 issues found in llvm repository:

• llvm/lib/Target/AMDGPU/AMDGPULibFunc.cpp:721:22: warning: reference variable 'S' extends the lifetime of a just-constructed temporary object 'const StringRef', consider changing reference to value [readability-reference-to-constructed-temporary]
• clang/lib/Sema/SemaChecking.cpp:10096:36: warning: reference variable 'AT2' extends the lifetime of a just-constructed temporary object 'const analyze_printf::ArgType', consider changing reference to value [readability-reference-to-constructed-temporary]

[Eugene.Zelenko]

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

23

const auto *.

27

Ditto.

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

32

Excessive newline.

[PiotrZSL]

Review comment fixes

[PiotrZSL]

Ping + Rebase

[PiotrZSL]

Ping

[PiotrZSL]

Ping

[PiotrZSL]

Ping

[xgupta]

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

21

A comment might be good to describe this struct.

23

This can be written as const auto *Other = Nodes.getNodeAs<ValueDecl>(ID); following other patterns.

40

Can be written using direct initialization as NotExtendedByDeclBoundToPredicate Predicate{ID};

64

This part looks like boilerplate in the middle of a specific implementation., isLanguageVersionSupported is mostly in the header file.

70

This part look like boilerplate in the middle of a specific implementation., getCheckTraversalKind() is mostly in the header file.

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

11

May be good to mention dangling references and resource leakage as potential issues.

19

The below comment is not matching, do you want to write -
const std::string& str = std::string("hello");
?

[PiotrZSL]

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

40

Yes, but I followed like others are done.

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

19

actually comment is +- fine, constructor to std::string will be called anyway, and for compiler I think those 2 are equal.
Will verify.

[PiotrZSL]

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

11

There will be no dangling references and resource leakage because lifetime of temporary is extended by variable.

[PiotrZSL]

Update

[xgupta]

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

19

but you have written below reference variable str is assigned a temporary object and there is no str variable.

[PiotrZSL]

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

19

Ok, that's actually a bug. It's value.

[PiotrZSL]

Rename value to str in doc.

[xgupta]

This LGTM, I will wait for two days before accepting the revision in case other reviewers might have some more comments/suggestions.

[xgupta]

LGTM!

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

add_clang_library(clangTidyReadabilityModule
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantPreprocessorCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantStringInitCheck.cpp
+  ReferenceToConstructedTemporaryCheck.cpp
   SimplifyBooleanExprCheck.cpp
   SimplifySubscriptExprCheck.cpp
   StaticAccessedThroughInstanceCheck.cpp
   StaticDefinitionInAnonymousNamespaceCheck.cpp
   StringCompareCheck.cpp
   SuspiciousCallArgumentCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp

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

 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
+#include "ReferenceToConstructedTemporaryCheck.h"
 #include "SimplifyBooleanExprCheck.h"
 #include "SimplifySubscriptExprCheck.h"
 #include "StaticAccessedThroughInstanceCheck.h"
 #include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "StringCompareCheck.h"
 #include "SuspiciousCallArgumentCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
         "readability-redundant-access-specifiers");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
         "readability-redundant-function-ptr-dereference");
     CheckFactories.registerCheck<RedundantMemberInitCheck>(
         "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>(
         "readability-static-accessed-through-instance");
     CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
         "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<StringCompareCheck>(
         "readability-string-compare");

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

+//===--- 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/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp

+//===--- 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/docs/ReleaseNotes.rst

- New :doc:`bugprone-multi-level-implicit-pointer-conversion
   indirection.
 
 - New :doc:`performance-enum-size
   <clang-tidy/checks/performance/enum-size>` check.
 
   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
 ^^^^^^^^^^^^^^^^^
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Removed checks
 ^^^^^^^^^^^^^^

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

.. csv-table::
    `readability-redundant-control-flow <readability/redundant-control-flow.html>`_, "Yes"
    `readability-redundant-declaration <readability/redundant-declaration.html>`_, "Yes"
    `readability-redundant-function-ptr-dereference <readability/redundant-function-ptr-dereference.html>`_, "Yes"
    `readability-redundant-member-init <readability/redundant-member-init.html>`_, "Yes"
    `readability-redundant-preprocessor <readability/redundant-preprocessor.html>`_,
    `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"
    `readability-static-definition-in-anonymous-namespace <readability/static-definition-in-anonymous-namespace.html>`_, "Yes"
    `readability-string-compare <readability/string-compare.html>`_, "Yes"
    `readability-suspicious-call-argument <readability/suspicious-call-argument.html>`_,
    `readability-uniqueptr-delete-release <readability/uniqueptr-delete-release.html>`_, "Yes"
    `readability-uppercase-literal-suffix <readability/uppercase-literal-suffix.html>`_, "Yes"

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

+.. 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

+// 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};
+}