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

Piotr Zegar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 26 11:12:09 PDT 2023


PiotrZSL added a comment.

In D144347#4222534 <https://reviews.llvm.org/D144347#4222534>, @ccotter wrote:

> Is it worth adding a cppcoreguidelines alias (ES.56)?

Yes, you can pull this change (arc patch D144347 <https://reviews.llvm.org/D144347> --nobranch) and create change that would depend on this one, and would register alias, also there you could override DisableRemoveSuggestion.
You can also help reviewing this check, to push it forward, point missing tests, scenarios. We could merge tests...



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst:98
+
+.. option:: DisableTypeMismatchSuggestion
+
----------------
ccotter wrote:
> Curious what others thing but I think the tool should by default not remove or replace `forward` with static_cast.  When an author has written `forward`, there is a good chance they intended to forward something (e.g., they forgot to use `&&` in the parameter declaration). My hypothesis is that it's more likely the author intended to forward something, rather than than they were trying to use it as a cast (but had not intention of forward - why else would they have typed it out?). In this case, I think it merits manual review by default (so the tool should always warn, but without the fixits by default).
No, in this case if they used forward, they used this with different type, not adding & is not sufficient to trigger this case.

In theory we could, create alias bugprone-std-forward that would redirect to this check, but would set DisableTypeMismatchSuggestion to false, and would set DisableRemoveSuggestion & DisableMoveSuggestion to true.

In such case we could have readability check that would just suggest remove of std::forward or usage of std::move, and bugprone check that would only warn of casts between different types.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144347



More information about the cfe-commits mailing list