[clang-tools-extra] 87d0aed - [clang-tidy] Add readability-reference-to-constructed-temporary check
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 29 03:39:27 PDT 2023
Author: Piotr Zegar
Date: 2023-07-29T10:39:17Z
New Revision: 87d0aedaa285cfca92aef67367ce55c476cf444e
URL: https://github.com/llvm/llvm-project/commit/87d0aedaa285cfca92aef67367ce55c476cf444e
DIFF: https://github.com/llvm/llvm-project/commit/87d0aedaa285cfca92aef67367ce55c476cf444e.diff
LOG: [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
Added:
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp
clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 421698cd615f9d..ba83cb441845c5 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index f95fa636f15774..b8e6e641432060 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/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>(
diff --git a/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp b/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp
new file mode 100644
index 00000000000000..587ae8ea305802
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h b/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h
new file mode 100644
index 00000000000000..c1f4f1c4d47dd6
--- /dev/null
+++ b/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
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d8c55c17be3e2f..a93ff58c60e2c0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/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
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 73aad3c0315ab0..8918899bbf418f 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/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"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst
new file mode 100644
index 00000000000000..09fd0c735fe782
--- /dev/null
+++ b/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
diff icult 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.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp
new file mode 100644
index 00000000000000..215b09a180537b
--- /dev/null
+++ b/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};
+}
More information about the cfe-commits
mailing list