[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

maybe some other name for check, like missing-std-forward.



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:62
+              namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),
+          argumentCountIs(1),
+          hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")))
----------------
move this up to be checkered first


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:67
+  Finder->addMatcher(
+      parmVarDecl(
+          parmVarDecl().bind("param"), isTemplateTypeOfFunction(),
----------------
anyway invert this logic.
would be nice to invert this to start matching with functionDecl, but we don't have forEachParameter


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:69-72
+          anyOf(hasAncestor(cxxConstructorDecl(
+                    ToParam, isDefinition(),
+                    optionally(hasDescendant(ForwardCallMatcher)))),
+                hasAncestor(functionDecl(
----------------
use isDefinition also for functionDecl,
anyway maybe
```
hasAncestor(functionnDecl(isDefinition(),
                                            ToParam,
                                            unless(hasDescendant(ForwardCallMatcher)))))))
```
I don't see reason for eplicit handling of cxxConstructorDecl, hasDescendant will match also initialization list.
Exclude code that is used in decltype, like:

```
template<typename T>
void test(T&& t) {
   using Type = decltype(callSomething(std::forward<T>(t)));
   callOther<Type>(t);
}
```


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.cpp:83-84
+
+  if (!Param)
+    return;
+
----------------
I thing this can never happen


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ForwardingReferenceParamNotForwardedCheck.h:32
+  }
+};
+
----------------
add here that thing about UnlessSpelledInSource....


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146921