[PATCH] D141569: [clang-tidy] Implement CppCoreGuideline F.18

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

  > If you add std::move you will get compilation error, if you add std::forward, everything will work fine. Simply Value& and && will evaluate to Value&, no rvalue reference here.

I agree that examining the template definition alone is not correct. In your original example with `SomeClass`, `T` is not a forwarding reference since `T` is not deduced in the constructor, so it's misleading to use `forward` in this context. `forward` "works" and leads to the behavior you are likely seeking in your `SomeClass` example, but not in the usual way the most readers would expect (the spirit of the CppCoreGuidelines is to provide certain restricted ideas as recommendations, and using `forward` in this way violates ES.56: "Flag when std::forward is applied to other than a forwarding reference."). Usually, I'd expect to see a class template on a type to have different specializations for `T` being a non-reference and another for `T` being a reference (e.g., `std::future`), or only allow `T` to be a reference or non-reference (e.g., `std::optional`), but not both.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141569/new/

https://reviews.llvm.org/D141569