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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits