diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -39,6 +39,7 @@ 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 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -42,6 +42,7 @@ #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "RedundantStringInitCheck.h" +#include "ReferenceToConstructedTemporaryCheck.h" #include "SimplifyBooleanExprCheck.h" #include "SimplifySubscriptExprCheck.h" #include "StaticAccessedThroughInstanceCheck.h" @@ -110,6 +111,8 @@ "readability-redundant-member-init"); CheckFactories.registerCheck( "readability-redundant-preprocessor"); + CheckFactories.registerCheck( + "readability-reference-to-constructed-temporary"); CheckFactories.registerCheck( "readability-simplify-subscript-expr"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h b/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h new file mode 100644 --- /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 getCheckTraversalKind() const override; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REFERENCETOCONSTRUCTEDTEMPORARYCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp b/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp @@ -0,0 +1,84 @@ +//===--- 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 { + +struct NotExtendedByDeclBoundToPredicate { + bool operator()(const internal::BoundNodesMap &Nodes) const { + const auto *Other = Nodes.getNode(ID).get(); + if (!Other) + return true; + + const auto *Self = Node.get(); + if (!Self) + return true; + + return Self->getExtendingDecl() != Other; + } + + StringRef ID; + ::clang::DynTypedNode Node; +}; + +AST_MATCHER_P(MaterializeTemporaryExpr, isExtendedByDeclBoundTo, StringRef, + ID) { + NotExtendedByDeclBoundToPredicate Predicate; + Predicate.ID = ID; + Predicate.Node = ::clang::DynTypedNode::create(Node); + return Builder->removeBindings(Predicate); +} + +} // namespace + +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); +} + +bool ReferenceToConstructedTemporaryCheck::isLanguageVersionSupported( + const LangOptions &LangOpts) const { + return LangOpts.CPlusPlus; +} + +std::optional +ReferenceToConstructedTemporaryCheck::getCheckTraversalKind() const { + return TK_AsIs; +} + +void ReferenceToConstructedTemporaryCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = Result.Nodes.getNodeAs("var"); + const auto *MatchedTemporary = Result.Nodes.getNodeAs("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/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -133,6 +133,12 @@ Checks that all implicit and explicit inline functions in header files are tagged with the ``LIBC_INLINE`` macro. +- New :doc:`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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -363,6 +363,7 @@ `readability-redundant-smartptr-get `_, "Yes" `readability-redundant-string-cstr `_, "Yes" `readability-redundant-string-init `_, "Yes" + `readability-reference-to-constructed-temporary `_, `readability-simplify-boolean-expr `_, "Yes" `readability-simplify-subscript-expr `_, "Yes" `readability-static-accessed-through-instance `_, "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 --- /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& value("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. 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 --- /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}; +}