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

2023-07-29 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-07-27 Thread Shivam Gupta via Phabricator via cfe-commits
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

2023-07-25 Thread Shivam Gupta via Phabricator via cfe-commits
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

2023-07-25 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-07-25 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-07-25 Thread Shivam Gupta via Phabricator via cfe-commits
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

2023-07-25 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-07-25 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-07-25 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-07-25 Thread Shivam Gupta via Phabricator via cfe-commits
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

2023-07-01 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-05-22 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-04-14 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-03-19 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-03-19 Thread Eugene Zelenko via Phabricator via cfe-commits
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

2023-03-18 Thread Piotr Zegar via Phabricator via cfe-commits
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

2023-03-18 Thread Piotr Zegar via Phabricator via cfe-commits
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 `_,