[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

[Chris Cotter via Phabricator via cfe-commits]

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