aaron.ballman added inline comments.
================
Comment at: clang-tidy/modernize/AvoidFunctionalCheck.cpp:21
+void AvoidFunctionalCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ cxxRecordDecl(allOf(anyOf(isDerivedFrom("std::binary_function"),
----------------
I don't think this matcher catches other uses of these types. e.g.,
```
typedef std::binary_function<int, int, bool> awesome_sauce;
struct S : awesome_sauce {};
template <typename Ty, typename = void>
struct S {};
template <typename Ty>
struct S<Ty, typename std::enable_if<std::is_base_of<std::binary_function<int,
int, bool>, Ty>::value>::type> {
typedef int value;
};
```
================
Comment at: clang-tidy/modernize/AvoidFunctionalCheck.cpp:22
+ Finder->addMatcher(
+ cxxRecordDecl(allOf(anyOf(isDerivedFrom("std::binary_function"),
+ isDerivedFrom("std::unary_function")),
----------------
These should all be `::std::` instead of `std::` to cover pathological code
that puts `std` inside of another namespace.
================
Comment at: clang-tidy/modernize/AvoidFunctionalCheck.cpp:37
+void AvoidFunctionalCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto* const UnaryOrBinaryDerived =
+ Result.Nodes.getNodeAs<RecordDecl>("un_or_binary_derived")) {
----------------
Formatting (here and elsewhere); you should run the patch through clang-format
to handle this sort of thing.
================
Comment at: clang-tidy/modernize/AvoidFunctionalCheck.cpp:43
+ const auto *ClangDecl = BaseType->getAsCXXRecordDecl();
+ if (!ClangDecl) {
+ continue;
----------------
Should elide the braces here.
================
Comment at: clang-tidy/modernize/AvoidFunctionalCheck.cpp:47-51
+ if (!ClangDecl->getDeclName().isIdentifier() ||
+ (ClangDecl->getName() != "unary_function" &&
+ ClangDecl->getName() != "binary_function")) {
+ continue;
+ }
----------------
Shouldn't this be an `assert` instead?
================
Comment at: clang-tidy/modernize/AvoidFunctionalCheck.h:19
+
+/// Check for several deprecated types and classes from <functional> header
+///
----------------
Missing full stop at the end of the sentence.
Why should this modernize check be limited to `<functional>`? Just like we have
a "deprecated headers" check, perhaps this should be a "deprecated APIs" check?
================
Comment at: docs/clang-tidy/checks/modernize-avoid-functional.rst:6
+
+In C++17 several classes, types and functions from <functional> header are no
longer available.
+In particular, this check warns if one of the following deprecated objects is
----------------
"In C++17, several classes, types, and functions" (commas).
https://reviews.llvm.org/D42730
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
