[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
This revision was automatically updated to reflect the committed changes. Closed by commit rG87d0aedaa285: [clang-tidy] Add readability-reference-to-constructed-temporary check (authored by PiotrZSL). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst 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 Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp === --- /dev/null +++ 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}; +} Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst === --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst === --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -380,6 +380,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" Index:
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
xgupta accepted this revision. xgupta added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
xgupta added a comment. This LGTM, I will wait for two days before accepting the revision in case other reviewers might have some more comments/suggestions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL updated this revision to Diff 544205. PiotrZSL marked an inline comment as done. PiotrZSL added a comment. Rename value to str in doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst 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 Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp === --- /dev/null +++ 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}; +} Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst === --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst === --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -379,6 +379,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" Index: clang-tools-extra/docs/ReleaseNotes.rst
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL marked 2 inline comments as done. PiotrZSL added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:18 + + const std::string& value("hello"); + xgupta wrote: > PiotrZSL wrote: > > xgupta wrote: > > > The below comment is not matching, do you want to write - > > > `const std::string& str = std::string("hello");` > > > ? > > actually comment is +- fine, constructor to std::string will be called > > anyway, and for compiler I think those 2 are equal. > > Will verify. > but you have written below `reference variable ``str`` is assigned a > temporary object` and there is no str variable. Ok, that's actually a bug. It's `value`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
xgupta added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:18 + + const std::string& value("hello"); + PiotrZSL wrote: > xgupta wrote: > > The below comment is not matching, do you want to write - > > `const std::string& str = std::string("hello");` > > ? > actually comment is +- fine, constructor to std::string will be called > anyway, and for compiler I think those 2 are equal. > Will verify. but you have written below `reference variable ``str`` is assigned a temporary object` and there is no str variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL updated this revision to Diff 544057. PiotrZSL marked an inline comment as done. PiotrZSL added a comment. Update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst 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 Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp === --- /dev/null +++ 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}; +} Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst === --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst === --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -379,6 +379,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" Index: clang-tools-extra/docs/ReleaseNotes.rst === ---
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL marked 4 inline comments as done. PiotrZSL added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:10 +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 xgupta wrote: > May be good to mention dangling references and resource leakage as potential > issues. There will be no dangling references and resource leakage because lifetime of temporary is extended by variable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:39 + ID) { + NotExtendedByDeclBoundToPredicate Predicate; + Predicate.ID = ID; xgupta wrote: > Can be written using direct initialization as > `NotExtendedByDeclBoundToPredicate Predicate{ID};` Yes, but I followed like others are done. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:18 + + const std::string& value("hello"); + xgupta wrote: > The below comment is not matching, do you want to write - > `const std::string& str = std::string("hello");` > ? actually comment is +- fine, constructor to std::string will be called anyway, and for compiler I think those 2 are equal. Will verify. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
xgupta added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:20 + +struct NotExtendedByDeclBoundToPredicate { + bool operator()(const internal::BoundNodesMap ) const { A comment might be good to describe this struct. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:22 + bool operator()(const internal::BoundNodesMap ) const { +const auto *Other = Nodes.getNode(ID).get(); +if (!Other) This can be written as `const auto *Other = Nodes.getNodeAs(ID);` following other patterns. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:39 + ID) { + NotExtendedByDeclBoundToPredicate Predicate; + Predicate.ID = ID; Can be written using direct initialization as `NotExtendedByDeclBoundToPredicate Predicate{ID};` Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:63 + +bool ReferenceToConstructedTemporaryCheck::isLanguageVersionSupported( +const LangOptions ) const { This part looks like boilerplate in the middle of a specific implementation., `isLanguageVersionSupported` is mostly in the header file. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:69 +std::optional +ReferenceToConstructedTemporaryCheck::getCheckTraversalKind() const { + return TK_AsIs; This part look like boilerplate in the middle of a specific implementation., getCheckTraversalKind() is mostly in the header file. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:10 +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 May be good to mention dangling references and resource leakage as potential issues. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst:18 + + const std::string& value("hello"); + The below comment is not matching, do you want to write - `const std::string& str = std::string("hello");` ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL updated this revision to Diff 513693. PiotrZSL added a comment. Ping + Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst 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 Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp === --- /dev/null +++ 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}; +} Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst === --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst === --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -366,6 +366,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" Index: clang-tools-extra/docs/ReleaseNotes.rst === ---
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL updated this revision to Diff 506399. PiotrZSL marked 3 inline comments as done. PiotrZSL added a comment. Review comment fixes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst 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 Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp === --- /dev/null +++ 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}; +} Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst === --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst === --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ 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" Index: clang-tools-extra/docs/ReleaseNotes.rst
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:22 + bool operator()(const internal::BoundNodesMap ) const { +auto *Other = Nodes.getNode(ID).get(); +if (!Other) `const auto *`. Comment at: clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp:26 + +auto Self = Node.get(); +if (!Self) Ditto. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp:31 +} + Excessive newline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL added a comment. 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] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146368/new/ https://reviews.llvm.org/D146368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check
PiotrZSL created this revision. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. PiotrZSL requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. 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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D146368 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.cpp clang-tools-extra/clang-tidy/readability/ReferenceToConstructedTemporaryCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst 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 Index: clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp === --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/readability/reference-to-constructed-temporary.cpp @@ -0,0 +1,31 @@ +// 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}; +} + Index: clang-tools-extra/docs/clang-tidy/checks/readability/reference-to-constructed-temporary.rst === --- /dev/null +++ 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. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst === --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ 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 `_,