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

[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 &Nodes) 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 &Nodes) const {
+    const auto *Other = Nodes.getNode(ID).get<ValueDecl>();
+    if (!Other)
----------------
This can be written as `const auto *Other = Nodes.getNodeAs<ValueDecl>(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 &LangOpts) 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<TraversalKind>
+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