ccotter added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp:99 + unless(isExpansionInSystemHeader()), argumentCountIs(1U), + IgnoreDependentExpresions + ? expr(unless(isInstantiationDependent())) ---------------- I think we might need more tests when `IgnoreDependentExpresions` is true. When I was playing around and hardcoded IgnoreDependentExpresions to false on line 99, the tests still pass. ================ Comment at: clang-tools-extra/docs/ReleaseNotes.rst:126 + + Suggests removing or replacing ``std::forward`` with ``std::move`` or + ``static_cast`` in cases where the template argument type is invariant and ---------------- Would you see use in expanding use of this check to cases that (broadly, cppcoreguiideline ES.56) apply `forward` when the argument is not a forwarding reference? Or separate check (as the one I proposed in https://reviews.llvm.org/D146888)? Specifically, when the template parameter is deduced: ``` template <class T> void forwards_incorrectly(T& t) { T other = std::forward<T>(t); } // ignored by readability-forward-usage ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144347/new/ https://reviews.llvm.org/D144347 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
