[PATCH] D146921: [clang-tidy] Implement cppcoreguidelines F.19
Piotr Zegar via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 26 11:43:49 PDT 2023
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
More information about the cfe-commits
mailing list