alexfh added a comment. A few nits. Otherwise looks good, if Sam has no objections.
================ Comment at: clang-tidy/performance/UnnecessaryValueParamCheck.cpp:34 @@ -29,1 +33,3 @@ +template <typename S> bool isSetDifferenceEmpty(const S &S1, const S &S2) { + for (const auto &E : S1) ---------------- +1 to `isSubset`. And please add a comment explaining what should be a subset of what for this function to return `true`. ================ Comment at: clang-tidy/utils/Matchers.h:50 @@ +49,3 @@ +AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isReferenceToConst) { + return ast_matchers::referenceType(ast_matchers::pointee( + ast_matchers::qualType(ast_matchers::isConstQualified()))); ---------------- How about `using ast_matchers;` in this function to make the statement easier to read? ================ Comment at: clang-tidy/utils/TypeTraits.cpp:129 @@ +128,3 @@ + auto *Record = Type->getAsCXXRecordDecl(); + if (!Record || !Record->hasDefinition()) + return false; ---------------- Maybe simplify this a bit? return Record && Record->hasDefinition() && Record->hasNonTrivialMoveAssignment(); ================ Comment at: clang-tidy/utils/TypeTraits.cpp:136 @@ +135,3 @@ + auto *Record = Type->getAsCXXRecordDecl(); + if (!Record || !Record->hasDefinition()) + return false; ---------------- ditto ================ Comment at: clang-tidy/utils/TypeTraits.h:32 @@ -31,1 +31,3 @@ +// Returns true if Type has a non-trivial move constructor. +bool hasNonTrivialMoveConstructor(QualType Type); ---------------- Please use doxygen comments (`///`) and enclose inline code snippets in backquotes. The other function comments in this file should also be doxygen, I've fixed them in r272994. http://reviews.llvm.org/D20277 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits