massberg marked 8 inline comments as done.
massberg added a comment.
Thanks for the comments!
================
Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:26
+ .bind("base")),
+ anyOf(isClass(), ast_matchers::isStruct()),
+ ast_matchers::isDefinition()))
----------------
aaron.ballman wrote:
> You can drop the `ast_matchers::` here because of the using declaration above.
>
> Actually, do we need the `isClass()` and `isStruct()` check at all? What else
> would match `isDerivedFrom()`?
Done.
You are right, as we use isDerivedFrom() we do not need isClass() and
isStruct() anymore.
================
Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:33
+ this);
+ Finder->addMatcher(callExpr(callee(functionDecl(hasName("std::mem_fun"))))
+ .bind("mem_fun_call"),
----------------
aaron.ballman wrote:
> ::std::mem_fun
>
> Why not `::std::mem_fun_ref`? Or the _t variants of these APIs?
There are several other types and functions in <functional> which are removed
in C++17. I will add them in a follow-up change.
This change just introduces the check with a limited number of cases and helps
me to get used to the clang-tidy coding style. ;)
================
Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+ } else if (const auto *const Call =
+ Result.Nodes.getNodeAs<CallExpr>("ptr_fun_call")) {
+ diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+ } else if (const auto *const Call =
+ Result.Nodes.getNodeAs<CallExpr>("mem_fun_call")) {
+ diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+ }
----------------
aaron.ballman wrote:
> I think that this code should be generalized (same with the matchers) so that
> you match on `hasAnyName()` for the function calls and use
> `CallExpr::getCalleeDecl()` to get the declaration. e.g.,
> ```
> if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("blech")) {
> if (const Decl *Callee = Call->getCalleeDecl())
> diag(Call->getLocStart(), Message) << Calleee;
> }
> ```
> This way you can add more named without having to add extra logic for the
> diagnostics.
I generalized the code and the matcher.
When we use
```
<< cast<NamedDecl>(Callee);
```
we get the template arguments in the name , e.g. `ptr_fun<int, int>`, so I
chose to use `getQualifiedNameAsString`.
If there is there a better way to get the function name without template
arguments I appreciate any suggestions.
https://reviews.llvm.org/D42730
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
